feat: Enable sorting of workbook order in Kanban board (#943)

[KATO-Hiro]

close #943

Summary by CodeRabbit

• New Features

  • Admin Kanban for reordering problem sets (drag‑and‑drop), persistent saves, initialization action, and guarded validation to prevent invalid moves.
  • Workbook placement: placement model, seeding, classification by grade/category, and APIs for batch updates.

• Tests

  • Extensive unit and E2E tests covering placement logic, Kanban utilities, server API error cases, and seed/init flows.

• Documentation

  • New/updated guides: architecture, coding style, testing, refactor planning, contribution notes, and implementation plan for ordering.

[coderabbitai]

Actionable comments posted: 1

♻️ Duplicate comments (1)

AGENTS.md (1)

58-63: ⚠️ Potential issue | 🟡 Minor

Test-location guidance is still internally inconsistent.

Line 58 says tests are co-located and “not in src/test/,” but Line 63 still documents src/test/ for unit tests. Clarify whether both are supported (and when), or remove one to make this unambiguous.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@AGENTS.md` around lines 58 - 63, The test-location guidance is inconsistent:
update AGENTS.md to either (A) explicitly state that co-located tests (e.g.,
_.test.ts next to source files) are the preferred pattern and remove the mention
of src/test/, or (B) document both patterns and when to use each — e.g.,
"co-located _.test.ts for feature/unit tests; use src/test/ only for large
integration suites or legacy tests" — and ensure every mention of "_.test.ts"
and "src/test/" in the document is adjusted to match the chosen single,
unambiguous policy.

🧹 Nitpick comments (1)

AGENTS.md (1)

58-62: Use an explicit test filename glob in the structure example.

_.test.ts is ambiguous in docs; use *.test.ts (or *.spec.ts) to communicate the intended naming pattern clearly.

Suggested doc tweak

-│ │ └── _.test.ts # Tests co-located next to source (not in src/test/)
+│ │ └── *.test.ts # Tests co-located next to source (not in src/test/)

-│ └── _.test.ts # Tests co-located next to source
+│ └── *.test.ts # Tests co-located next to source

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@AGENTS.md` around lines 58 - 62, Update the example filesystem snippet in
AGENTS.md to use an explicit test filename glob instead of the ambiguous literal
`_.test.ts`; replace instances of `_.test.ts` with a glob like `*.test.ts` (or
`*.spec.ts`) so the intended naming pattern is clear in the docs.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@AGENTS.md`:
- Line 12: Decide on a single contributor workflow (either TDD or "production
code then tests") and make both occurrences consistent: replace the sentence
"Implement production code → tests → verify with `pnpm test:unit`" and the
sentence "Write tests before implementation (TDD)" with the chosen unified rule;
for example, if you choose TDD, update both to "Write tests before
implementation (TDD) → implement production code → verify with `pnpm
test:unit`", and if you choose the other flow, update both to "Implement
production code → write tests → verify with `pnpm test:unit`", ensuring the two
unique phrases in the document are identical.

---

Duplicate comments:
In `@AGENTS.md`:
- Around line 58-63: The test-location guidance is inconsistent: update
AGENTS.md to either (A) explicitly state that co-located tests (e.g., _.test.ts
next to source files) are the preferred pattern and remove the mention of
src/test/, or (B) document both patterns and when to use each — e.g.,
"co-located _.test.ts for feature/unit tests; use src/test/ only for large
integration suites or legacy tests" — and ensure every mention of "_.test.ts"
and "src/test/" in the document is adjusted to match the chosen single,
unambiguous policy.

---

Nitpick comments:
In `@AGENTS.md`:
- Around line 58-62: Update the example filesystem snippet in AGENTS.md to use
an explicit test filename glob instead of the ambiguous literal `_.test.ts`;
replace instances of `_.test.ts` with a glob like `*.test.ts` (or `*.spec.ts`)
so the intended naming pattern is clear in the docs.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7ee20b44-e275-4586-ad40-7e66352f6bf3

📥 Commits

Reviewing files that changed from the base of the PR and between 33344d2 and 28cdf7c.

📒 Files selected for processing (1)

• AGENTS.md

[coderabbitai]

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.claude/rules/prisma-db.md:
- Around line 62-63: Update the documentation path referenced in the rule text:
replace the incorrect "docs/erd.md" mention with the actual tracked file
"prisma/ERD.md" so contributors are directed to the correct ERD document; ensure
the sentence "add a comment to `docs/erd.md` under the relevant entity" is
changed to reference `prisma/ERD.md` wherever it appears in the rule (e.g., the
line containing that phrase).
- Around line 64-66: The fenced code block containing the comment "%% XOR
constraint: workbookplacement_xor_grade_category — exactly one of taskGrade or
solutionCategory must be non-null" is missing a language tag; update that
triple-backtick fence to include a language (e.g., text) so markdownlint MD040
stops flagging it, i.e., edit the fence surrounding the "%% XOR constraint:
workbookplacement_xor_grade_category" line to be ```text instead of ``` while
leaving the content unchanged.

In `@docs/dev-notes/2026-02-28/workbook-order/plan.md`:
- Around line 162-163: The TODOs are outdated: remove or mark done the two
checklist items—(1) the auth consolidation task referencing validateAdminAccess
since auth is already centralized in src/routes/(admin)/_utils/auth.ts (ensure
the note mentions validateAdminAccess in _utils/auth.ts), and (2) the placement
initialization task referencing initializePlacements in +page.server.ts because
the page already calls createInitialPlacements() from the service layer; update
the checklist to reflect both as completed or delete them so the plan no longer
misleads readers.

In `@src/features/workbooks/services/workbook_placements.ts`:
- Around line 143-154: buildCurriculumWorkbooksForInit currently injects an
empty-string sentinel for missing task IDs which can break later assumptions
(see gradeModes.get(...) usage); update the function to filter out workBookTask
entries that lack a real task/task_id (use workBookTask.task?.task_id) instead
of mapping to '' and either set workBookTasks to the filtered array or throw a
clear error if a workbook ends up with zero valid tasks so downstream code
(e.g., where gradeModes.get is called) never receives a '' sentinel; refer to
buildCurriculumWorkbooksForInit, workBookTasks, and task?.task_id when locating
the change.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: dac1ed8a-7a17-45be-8e33-db2fa8054614

📥 Commits

Reviewing files that changed from the base of the PR and between 28cdf7c and 102f6d5.

📒 Files selected for processing (5)

• .claude/rules/prisma-db.md
• AGENTS.md
• docs/dev-notes/2026-02-28/workbook-order/plan.md
• src/features/workbooks/services/workbook_placements.test.ts
• src/features/workbooks/services/workbook_placements.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• src/features/workbooks/services/workbook_placements.test.ts

[coderabbitai]

🧹 Nitpick comments (1)

.claude/rules/testing.md (1)

80-86: Consider a more generic mock-helper example.

Line 81 currently uses WorkBookPlacements, which makes this global rule feel domain-specific. A generic type parameter would make the guidance reusable across modules.

Suggested doc snippet refinement

-function mockFindMany(value: WorkBookPlacements) {
+function mockFindMany<T>(value: T) {
   vi.mocked(prisma.workBookPlacement.findMany).mockResolvedValue(
     value as unknown as Awaited<ReturnType<typeof prisma.workBookPlacement.findMany>>,
   );
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.claude/rules/testing.md around lines 80 - 86, The helper mockFindMany uses
a concrete WorkBookPlacements type making the rule domain-specific; change it to
a generic signature like function mockFindMany<T>(value: T) so it can accept any
shape, and keep the mocked call the same but cast the generic value to the
awaited return type (e.g.,
vi.mocked(prisma.workBookPlacement.findMany).mockResolvedValue(value as unknown
as Awaited<ReturnType<typeof prisma.workBookPlacement.findMany>>)); update any
usages accordingly so this helper is reusable across modules.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In @.claude/rules/testing.md:
- Around line 80-86: The helper mockFindMany uses a concrete WorkBookPlacements
type making the rule domain-specific; change it to a generic signature like
function mockFindMany<T>(value: T) so it can accept any shape, and keep the
mocked call the same but cast the generic value to the awaited return type
(e.g., vi.mocked(prisma.workBookPlacement.findMany).mockResolvedValue(value as
unknown as Awaited<ReturnType<typeof prisma.workBookPlacement.findMany>>));
update any usages accordingly so this helper is reusable across modules.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e84cebbd-1a4b-491e-9185-86c892fde600

📥 Commits

Reviewing files that changed from the base of the PR and between 102f6d5 and bf4fca2.

📒 Files selected for processing (4)

• .claude/rules/coding-style.md
• .claude/rules/prisma-db.md
• .claude/rules/testing.md
• docs/dev-notes/2026-02-28/workbook-order/plan.md

🚧 Files skipped from review as they are similar to previous changes (2)

• docs/dev-notes/2026-02-28/workbook-order/plan.md
• .claude/rules/prisma-db.md