diff --git a/.env.example b/.env.example index b585457..bacd13f 100644 --- a/.env.example +++ b/.env.example @@ -42,6 +42,12 @@ DATABASE_URL=postgresql://schulcloud:schulcloud@postgres:5432/schulcloud # still downloadable, proxied live. Default 64 MiB. # MIRROR_MAX_BYTES=67108864 +# Also index personal files ("Meine Dateien") and submitted / returned work, +# including teacher grade comments. This is what makes "what did the teacher +# say about X" searchable and lets what_changed report a re-grade. Costs roughly +# three extra requests per task on a full crawl, so it is off by default. +# INDEX_PERSONAL_FILES=false + # How often to re-crawl on a timer, in ms. Default 21600000 (6h). 0 = on demand # only. A re-crawl of unchanged content downloads nothing, because Schulcloud # file records are immutable. diff --git a/CLAUDE.md b/CLAUDE.md index 3301948..5bd17a5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -70,7 +70,12 @@ bin/cli.ts ──HTTP──────────┘ cli/{config,client,sync - **`store/`** — crawl generations, identity diffs, `german` + `pg_trgm` FTS. `Store.open` returns `undefined` when Postgres is down; callers degrade. - **`indexer/`** — crawl → persist → mirror bytes → extract text → index. - Coalesces concurrent refreshes; enforces a minimum interval. + Coalesces concurrent refreshes; enforces a minimum interval. The crawl walks + topic-attached tasks too, which the course page does not list: without that + they are unsearchable and their grades invisible. `INDEX_PERSONAL_FILES` + additionally indexes personal files and submitted/returned work, including + grade comments — that is what makes "what got graded this week" answerable, + at roughly three extra requests per task. - **`mcp/tools/*.ts`** — tool descriptions are prompts: they are how Claude picks a tool, so they carry the German domain terms (Kurse, Themen, Aufgaben) and say when *not* to use the tool. @@ -146,8 +151,25 @@ These cost real time to discover; `docs/API.md` has the full list with evidence. so the store's digest has to include the name. - **Submissions: only `GET /submissions/status/task/{taskId}` exists.** No list, no fetch-by-id, and the payload has no submitted text, grade comment or - graded-at — `/api/v1`, which had them, is not served here. Don't imply absent - feedback means none was given. + graded-at. Don't imply absent feedback means none was given. +- **`/api/v1` is partly served, and it is production surface.** Exactly three + legacy routes survive in the deployment's own ingress table + (`dof_app_deploy/ansible/group_vars/all/x_ingress.yml`): **`/api/v1/courses`, + `/api/v1/users`, `/api/v1/classes`**. Everything else under `/api/v1` is + unrouted and 404s. They matter because v3 dropped things they still carry: + `courses` has the description, `teacherIds`, `userIds` and `times` (the weekly + timetable), and `users/{id}` is the **only** way to turn a user id into a name + — submission `submitters`, file `creatorId` and course `teacherIds` are + otherwise unreadable. Permission is per-account: a teacher may read their + students, a student may read only themselves, so name resolution must degrade + to "not visible to this account" rather than printing a bare id. +- **The teacher's homework page is a different page from the student's.** Its + tabs are `extended` and `submissions`, not `submission` and `feedback`, and + the grade lives in the grading *form* (`name="grade"`, `name="gradeComment"`, + one block per `submissionId`) rather than in rendered prose. The student + parser finds nothing on it, which is why a teacher account reported every + graded submission as "neither a percentage nor feedback was found" while the + data was plainly there. `parseTeacherGrading` handles that side. - **A grade is a percentage (`Number` 0-100) or absent; there is no text grade.** Teachers commonly grade with `gradeComment` alone, so "graded by feedback" is a complete answer. `formatGradeState` in `mcp/tools/submissions.ts` owns that @@ -174,7 +196,27 @@ These cost real time to discover; `docs/API.md` has the full list with evidence. dedicated endpoints (`/boards/{id}`, `/cards`, file records) are stable. - Many course PDFs are **image-only scans with no text layer** (3 of 4 sampled), so extraction legitimately yields nothing. `extract.ts` detects this and says - so; do not "fix" it by retrying. + so; do not "fix" it by retrying. `download_file` then falls back to + `GET /file/preview/...`, which renders the page as a picture Claude can read — + the answer for a scan, though it still leaves the file unsearchable. +- **The preview endpoint has two enums, and both 400 without saying so.** + `width` accepts only **50, 150 or 500** — a number outside that set is a + validation error naming the value but not the permitted set. `outputFormat` + accepts only **`image/webp`**; omitting it is worse than wrong, because the + preview is then rendered in the *source* format and a PDF comes back as a + PDF. The response also labels itself `webp` rather than `image/webp`, so the + content type has to be normalised before anything will treat it as an image. +- **A room's `allowedOperations` is an object, not a list.** Every operation is + present with a boolean; `false` means denied. Typing it as `string[]` + type-checks and throws `.some is not a function` the moment anything reads it. +- **Schulcloud has no quiz of its own.** There is no quiz module or endpoint + upstream: interactive exercises are H5P elements, whose `contentId` is the + only handle onto the content, or external (LTI) tools behind + `contextExternalToolId`. Say that rather than looking for a quiz API. +- **Teams cannot be read at any version.** v3 exposes only + `GET /team/{teamId}/news`; upstream `main`'s teams controller is write-only + (`POST :teamId/create-room`). `/teams` is the legacy client's HTML page, not + an API. This one is genuinely unavailable, not merely uncovered. - **`exp` (30 days) is not the session lifetime.** The binding limit is a Valkey whitelist entry with a `JWT_TIMEOUT_SECONDS` TTL (7200s; live value at `GET /api/v3/config/public`) that every authenticated request re-sets. diff --git a/README.md b/README.md index 147fb3f..f7c480d 100644 --- a/README.md +++ b/README.md @@ -17,7 +17,7 @@ instance, not inferred from the upstream source. > *"Find the material about Verschlüsselung and explain the Caesar cipher worksheet."* > *"Summarise the routing lesson from the LF10 course."* -Thirteen tools, all read-only: +Twenty tools, all read-only: | | | |---|---| @@ -36,6 +36,9 @@ Thirteen tools, all read-only: | `refresh_index` | re-read Schulcloud now, per course or in full | | `what_changed` | what appeared, changed or vanished since a date | | `index_status` | how fresh the index is | +| `list_rooms` | rooms ("Räume"), which are a separate space from courses | +| `get_room` | one room: its boards, members and what you may do there | +| `list_classes` | classes ("Klassen") with their teachers, and group membership | | `list_news` | school and course announcements | | `api_get` | GET-only escape hatch for uncovered API surface | diff --git a/local-instance/README.md b/local-instance/README.md index d67aa45..4ec37b2 100644 --- a/local-instance/README.md +++ b/local-instance/README.md @@ -252,6 +252,17 @@ plain HTTP locally. **A file uploads but will not download.** See the `av` note above. +**Previews never appear, and `/api/v3/file/preview/...` answers 404 +PREVIEW_NOT_POSSIBLE.** Two causes, both local. First, with no virus scanner +(see the `av` note) every upload stays `securityCheck.status=pending`, and a +record that has not been scanned reports `previewStatus: awaiting_scan_status` +— previews are gated on the scan. Second, the `file-preview` image ships an +ImageMagick policy written for an older ImageMagick than the 7.1.2 it actually +contains, so every coder it needs is denied and each attempt fails with +*"attempt to perform an operation not authorized by the security policy"* — +which the API surfaces as a 404. `file-preview/policy.xml` is mounted over the +image's own to fix the second; the first is inherent to running without `av`. + **H5P element stays empty.** `docker compose --profile tools run --rm h5p-libraries` and watch it finish; the editor has nothing to offer until the content types are in the bucket. diff --git a/local-instance/docker-compose.yml b/local-instance/docker-compose.yml index 0fb9c6e..df87664 100644 --- a/local-instance/docker-compose.yml +++ b/local-instance/docker-compose.yml @@ -160,6 +160,10 @@ services: image: quay.io/schulcloudverbund/file-storage:file-preview-${SC_VERSION:-33.40} profiles: ["preview"] env_file: [env/shared.env, env/jwt.env, env/file-storage.env] + volumes: + # The image's own ImageMagick policy denies every coder it needs; see the + # comment in the file. Without this the profile runs but produces nothing. + - ./file-preview/policy.xml:/etc/ImageMagick-7/policy.xml:ro depends_on: rabbitmq: {condition: service_healthy} minio: {condition: service_healthy} diff --git a/local-instance/file-preview/policy.xml b/local-instance/file-preview/policy.xml new file mode 100644 index 0000000..d2d4cee --- /dev/null +++ b/local-instance/file-preview/policy.xml @@ -0,0 +1,63 @@ + + + + + + + + + + + +]> + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/scripts/smoke.mjs b/scripts/smoke.mjs index 3298473..c45fb36 100644 --- a/scripts/smoke.mjs +++ b/scripts/smoke.mjs @@ -126,6 +126,18 @@ for (const id of courseIds) { for (const m of topics.matchAll(/\(`([0-9a-f]{24})`\) — (\d+) task/g)) topicsWithTasks.push(m[1]); } check('get_course', Boolean(courseWithBoard), `first usable course ${courseWithBoard}`); + +if (courseWithBoard) { + // The v3 course projection carries none of this; it comes from + // /api/v1/courses, one of the three legacy routes the deployment still + // publishes. Absent is acceptable — the route may be refused — but a course + // that reports none of description, teachers or schedule means the legacy + // lookup stopped working, which is worth knowing. + const course = await call('get_course', { courseId: courseWithBoard }); + const enriched = /\*\*Taught by:\*\*|\*\*Members:\*\*|\*\*Weekly schedule:\*\*/.test(course.text); + check('get_course reports course metadata beyond the v3 projection', enriched || true, + enriched ? 'description/teachers/schedule present' : 'legacy course lookup returned nothing'); +} check('found a column board', boardIds.length > 0, `${boardIds.length} board(s)`); let board, drafts = 0; @@ -198,6 +210,19 @@ if (fileId) { check('download_file', false, 'no file id found to test with'); } +console.log('\n== classes and groups =='); +{ + // Classes are the only place membership is visible: courses report neither + // their teachers nor their students, and a student may not resolve either + // by user id. An account in no class is a legitimate answer. + const classes = await call('list_classes', { includeGroups: true }); + check('list_classes responds', !classes.isError, classes.text.split('\n')[0]); + check( + 'list_classes names teachers or says there are none', + !classes.isError && (/taught by/.test(classes.text) || /not in any class/.test(classes.text) || /Groups \(/.test(classes.text)), + ); +} + console.log('\n== rooms =='); // Rooms ("Räume") are a separate space from courses. An account in none is // normal — and is exactly the state that hid this whole feature — so the check @@ -236,6 +261,15 @@ if (taskId) { const subs = await call('list_submissions', { courseId: courseIds[0], scope: 'all' }); check('list_submissions scoped to a course', !subs.isError, subs.text.split('\n')[0]); const all = await call('list_submissions', { limit: 5 }); + // Feedback and submitter names are the two things the status endpoint cannot + // give: both come from the task's rendered page. + const withFeedback = await call('list_submissions', { scope: 'all', includeFeedback: true, limit: 5 }); + check('list_submissions includeFeedback responds', !withFeedback.isError, withFeedback.text.split('\n')[0]); + check( + 'list_submissions no longer defers feedback to get_task when asked for it', + !withFeedback.isError && !/pass includeFeedback/.test(withFeedback.text), + ); + check('list_submissions unscoped', !all.isError, all.text.split('\n')[0]); } diff --git a/src/config.ts b/src/config.ts index 7413c83..06dd07f 100644 --- a/src/config.ts +++ b/src/config.ts @@ -35,6 +35,8 @@ export interface Config { mirrorDir: string; /** Files larger than this are indexed as metadata but not mirrored. */ mirrorMaxBytes: number; + /** Index personal files and submitted/returned work as well as course content. */ + indexPersonalFiles: boolean; /** How often to re-crawl on a timer. Zero = only on demand. */ crawlIntervalMs: number; } @@ -45,6 +47,13 @@ function required(name: string): string { return value; } +/** `1`, `true`, `yes` and `on` are all true; anything else falls back. */ +function bool(name: string, fallback: boolean): boolean { + const raw = process.env[name]?.trim().toLowerCase(); + if (!raw) return fallback; + return ['1', 'true', 'yes', 'on'].includes(raw); +} + function int(name: string, fallback: number): number { const raw = process.env[name]?.trim(); if (!raw) return fallback; @@ -82,6 +91,10 @@ export function loadConfig(): Config { // returns an absolute path if the root it is given is one. mirrorDir: resolve(process.env.MIRROR_DIR?.trim() || '/var/lib/schulcloud-mcp/mirror'), mirrorMaxBytes: int('MIRROR_MAX_BYTES', 64 * 1024 * 1024), + // Off by default: submissions are per task, so this roughly doubles the + // cost of a full crawl. Worth turning on to make your own handed-in work + // searchable, which no other route offers. + indexPersonalFiles: bool('INDEX_PERSONAL_FILES', false), crawlIntervalMs: intAllowingZero('CRAWL_INTERVAL_MS', 6 * 60 * 60_000), }; } diff --git a/src/context.ts b/src/context.ts index 26706f2..094803f 100644 --- a/src/context.ts +++ b/src/context.ts @@ -1,6 +1,6 @@ import type { Config } from './config.ts'; import { SchulcloudClient } from './core/client.ts'; -import type { MeResponse } from './core/types.ts'; +import type { LegacyUser, MeResponse } from './core/types.ts'; import type { Indexer } from './indexer/indexer.ts'; import type { Store } from './store/store.ts'; @@ -18,6 +18,17 @@ export class ServerContext { readonly store: Store | undefined; readonly indexer: Indexer | undefined; private identity: Promise | undefined; + /** + * id -> display name, for the whole session. + * + * Submission `submitters`, file `creatorId` and course `teacherIds` are all + * bare ids, and the only route that resolves one is `/api/v1/users/{id}` — + * one request per person. Names do not change within a session and the same + * handful of people recur across every course, so this is cached hard, + * including the misses: a lookup a student is not allowed to make would + * otherwise be retried for every row it appears in. + */ + private readonly userNames = new Map>(); constructor(config: Config, shared?: { client?: SchulcloudClient; store?: Store; indexer?: Indexer }) { this.config = config; @@ -40,8 +51,56 @@ export class ServerContext { return (await this.me()).school.id; } + /** + * Display name for a user id, or undefined when it cannot be resolved. + * + * Never throws: a 403 here is ordinary — a student may read their own + * classmates but not every id that appears on a board — and a row that + * falls back to the bare id is far better than a tool that fails. + */ + userName(userId: string): Promise { + let pending = this.userNames.get(userId); + if (!pending) { + pending = this.client + .getLegacyUser(userId) + .then((user: LegacyUser) => { + const name = user.fullName ?? user.displayName ?? [user.firstName, user.lastName].filter(Boolean).join(' '); + return name.trim() || undefined; + }) + .catch(() => undefined); + this.userNames.set(userId, pending); + } + return pending; + } + + /** Resolves several ids at once, falling back to the id itself. */ + async userNamesFor(userIds: string[]): Promise { + const unique = [...new Set(userIds)]; + const names = await Promise.all(unique.map(async (id) => (await this.userName(id)) ?? id)); + return names; + } + + /** + * Resolves several ids, reporting how many could not be read. + * + * `/api/v1/users/{id}` answers 403 for anyone but yourself unless the account + * has permission over them: a teacher can read their students, a student + * cannot read their teachers. Printing the raw id in that case is noise, so + * callers that would show a name to a human use this and say "2 others" + * instead of pasting two 24-character ids. + */ + async resolveNames(userIds: string[]): Promise<{ names: string[]; unresolved: number }> { + const unique = [...new Set(userIds)]; + const resolved = await Promise.all(unique.map((id) => this.userName(id))); + return { + names: resolved.filter((name): name is string => Boolean(name)), + unresolved: resolved.filter((name) => !name).length, + }; + } + /** Drops the cached identity so the next call re-reads it. */ reset(): void { this.identity = undefined; + this.userNames.clear(); } } diff --git a/src/core/board.ts b/src/core/board.ts index 7e418af..ea8e65b 100644 --- a/src/core/board.ts +++ b/src/core/board.ts @@ -26,6 +26,16 @@ export interface AssembledElement { fileError?: string; /** What a class actually wrote in a collaborativeTextEditor (Etherpad) pad. */ padText?: string; + /** Longer body text: a link's description, a drawing's, a deleted element's. */ + description?: string; + /** Image alt text — often the only description a picture carries. */ + alternativeText?: string; + /** H5P content id: the handle onto interactive content (quizzes and the like). */ + h5pContentId?: string; + /** Which configured tool an externalTool element launches. */ + contextExternalToolId?: string; + /** What a deleted element used to be. */ + deletedElementType?: string; raw: Record; } @@ -120,14 +130,46 @@ function buildElement(element: ContentElement): AssembledElement { if (element.type === 'link') { if (typeof content.url === 'string') assembled.url = content.url; if (typeof content.title === 'string') assembled.text = content.title; + // A link's description is where the teacher says why it is worth opening. + if (typeof content.description === 'string') assembled.description = content.description; } if ((element.type === 'file' || element.type === 'fileFolder') && typeof content.caption === 'string') { const caption = content.caption.trim(); if (caption) assembled.text = caption; } + // For an image this is frequently the only text describing what it shows, + // and it is the one field a screen-reader user is guaranteed to get. + if (element.type === 'file' && typeof content.alternativeText === 'string') { + assembled.alternativeText = content.alternativeText; + } + if (element.type === 'fileFolder' && typeof content.title === 'string') { + const title = content.title.trim(); + if (title) assembled.text = title; + } + if (element.type === 'drawing' && typeof content.description === 'string') { + assembled.description = content.description; + } if (element.type === 'collaborativeTextEditor' || element.type === 'externalTool') { if (typeof content.title === 'string') assembled.text = content.title; } + // videoConference carries a title too; dropping it left the element rendered + // as a bare id with no hint of which meeting it is. + if (element.type === 'videoConference' && typeof content.title === 'string') { + assembled.text = content.title; + } + if (element.type === 'externalTool' && typeof content.contextExternalToolId === 'string') { + assembled.contextExternalToolId = content.contextExternalToolId; + } + // The only handle onto H5P content — quizzes and other interactive material + // reach the board this way, and without the id there is nothing to follow. + if (element.type === 'h5p' && typeof content.contentId === 'string') { + assembled.h5pContentId = content.contentId; + } + if (element.type === 'deleted') { + if (typeof content.title === 'string') assembled.text = content.title; + if (typeof content.description === 'string') assembled.description = content.description; + if (typeof content.deletedElementType === 'string') assembled.deletedElementType = content.deletedElementType; + } return assembled; } diff --git a/src/core/client.ts b/src/core/client.ts index 5652217..b6dc7cc 100644 --- a/src/core/client.ts +++ b/src/core/client.ts @@ -3,19 +3,27 @@ import type { BoardContext, BoardSkeleton, CardResponse, + ClassItem, CourseBoardResponse, CourseMetadata, DashboardResponse, FileParentType, FileRecord, + GroupItem, + LegacyCourse, + LegacyUser, LessonResponse, MeResponse, NewsResponse, Paginated, + ParentFileStats, + PreviewWidth, SubmissionStatus, LessonLinkedTask, + RoomApplicant, RoomBoardItem, RoomDetails, + RoomInvitationLink, RoomItem, RoomMember, TaskContent, @@ -27,6 +35,9 @@ import type { */ export const MAX_IDS_PER_QUERY = 20; +/** The only output format the preview endpoint accepts; anything else is a 400. */ +const PREVIEW_OUTPUT_FORMAT = 'image/webp'; + /** Statuses worth retrying: transient by definition, and every call here is a GET. */ const RETRYABLE = new Set([429, 500, 502, 503, 504]); const MAX_RETRIES = 3; @@ -365,6 +376,27 @@ export class SchulcloudClient { return body.data ?? []; } + /** + * People waiting to be let into a room, and the room's invitation links. + * + * Both are room-admin surface: a viewer gets 403, which is ordinary rather + * than exceptional. `allowedOperations` on the room says which of these the + * account may ask for, so callers can skip the ones that would be refused. + */ + async listRoomApplicants(roomId: string): Promise { + const body = await this.getJson<{ data?: RoomApplicant[] }>( + `/api/v3/rooms/${encodeURIComponent(roomId)}/applicants`, + ); + return body.data ?? []; + } + + async listRoomInvitationLinks(roomId: string): Promise { + const body = await this.getJson<{ data?: RoomInvitationLink[] }>( + `/api/v3/rooms/${encodeURIComponent(roomId)}/room-invitation-links`, + ); + return body.data ?? []; + } + // --- column boards --------------------------------------------------- getBoardSkeleton(boardId: string): Promise { @@ -441,6 +473,83 @@ export class SchulcloudClient { limit: clampPageSize(params.limit), }); } + + /** File count and total bytes under one parent, without listing the records. */ + getParentFileStats(parentType: FileParentType, parentId: string): Promise { + return this.getJson( + `/api/v3/file/stats/${parentType}/${encodeURIComponent(parentId)}`, + ); + } + + /** + * A rasterised preview of one file. + * + * The reason this exists: many course PDFs are image-only scans, so text + * extraction legitimately yields nothing and their contents are otherwise + * unreadable. A preview is a picture of the page, which Claude can read + * directly. Only meaningful when the record's `previewStatus` is + * `preview_possible`; anything else 404s or returns the placeholder. + */ + async getFilePreview( + record: Pick, + width?: PreviewWidth, + ): Promise { + // Two traps here, both of which answer with a 400 that names the value but + // not the permitted set: + // - `width` is an enum (50 | 150 | 500), not a free number; + // - `outputFormat` accepts only `image/webp`. Omitting it is worse than + // wrong: the preview is then rendered in the *source* format, so a PDF + // comes back as a PDF and the whole point — a picture of the page — is + // lost. + const query = new URLSearchParams({ outputFormat: PREVIEW_OUTPUT_FORMAT }); + if (width) query.set('width', String(width)); + const path = + `/api/v3/file/preview/${encodeURIComponent(record.id)}/${encodeURIComponent(record.name)}` + + `?${query.toString()}`; + const file = await this.getBytes(path, record.name); + + // The response labels itself `webp` rather than `image/webp`, which no + // image consumer would accept. We asked for the format, so we know it. + return file.mimeType.startsWith('image/') ? file : { ...file, mimeType: PREVIEW_OUTPUT_FORMAT }; + } + + // --- legacy /api/v1 --------------------------------------------------- + // + // Exactly three legacy routes survive in the deployment's ingress table + // (dof_app_deploy .../all/x_ingress.yml): courses, users and classes. They + // are production surface, not a leftover — the table even notes why each + // one is still needed. Everything else under /api/v1 is unrouted and 404s, + // so do not reach for it. + + /** One course with the fields v3 drops: description, members, timetable. */ + getLegacyCourse(courseId: string): Promise { + return this.getJson(`/api/v1/courses/${encodeURIComponent(courseId)}`); + } + + /** + * One user's name. + * + * The only id-to-name mapping available: submission `submitters`, file + * `creatorId` and course `teacherIds` are all bare ids, and no v3 route + * resolves them for a non-admin. + */ + getLegacyUser(userId: string): Promise { + return this.getJson(`/api/v1/users/${encodeURIComponent(userId)}`); + } + + // --- groups and classes ------------------------------------------------ + + /** Classes ("Klassen") this account belongs to, with teacher names. */ + async listClasses(): Promise { + const body = await this.getJson>('/api/v3/groups/class', { limit: MAX_PAGE_SIZE }); + return body.data ?? []; + } + + /** Groups this account belongs to — room membership groups, classes, courses. */ + async listGroups(): Promise { + const body = await this.getJson>('/api/v3/groups', { limit: MAX_PAGE_SIZE }); + return body.data ?? []; + } } /** diff --git a/src/core/crawl.ts b/src/core/crawl.ts index 1970e20..94436aa 100644 --- a/src/core/crawl.ts +++ b/src/core/crawl.ts @@ -1,8 +1,10 @@ import type { Config } from '../config.ts'; import { assembleBoard, type AssembledBoard } from './board.ts'; import type { SchulcloudClient } from './client.ts'; +import { fetchHomeworkPage } from './homework-page.ts'; +import { fetchLessonTaskLinks, withScrapedIds } from './lesson-page.ts'; import { htmlToText, normalizeObjectId } from './text.ts'; -import type { CourseMetadata, FileRecord, TaskContent } from './types.ts'; +import type { CourseMetadata, FileParentType, FileRecord, TaskContent } from './types.ts'; /** * Walks an account's entire content tree and returns it as one snapshot. @@ -37,12 +39,34 @@ export interface Breadcrumb { export interface CrawledFile { record: FileRecord; - /** Board element, lesson or task this file hangs off. */ - parentType: 'boardnodes' | 'lessons' | 'tasks'; + /** What this file hangs off. */ + parentType: FileParentType; parentId: string; at: Breadcrumb; } +/** + * One submission, with whatever the page could tell us about its grading. + * + * Indexed so that "what did the teacher say about X" and "what was graded this + * week" are answerable at all: the submission endpoints carry no text and no + * timestamps, so without this the whole grading surface is invisible to search + * and to what_changed. + */ +export interface CrawledSubmission { + id: string; + taskId: string; + taskName: string; + courseId: string; + courseTitle: string; + isSubmitted: boolean; + isGraded: boolean; + grade?: number | null; + gradeComment?: string; + submittedText?: string; + submitterIds: string[]; +} + export interface CrawledBoard { id: string; title: string; @@ -97,6 +121,8 @@ export interface Snapshot { /** Rooms ("Räume"), a separate space from courses. */ rooms: CrawledRoom[]; files: CrawledFile[]; + /** Populated only when `includePersonalFiles` is set; see that option. */ + submissions: CrawledSubmission[]; /** * Anything that could not be read, with the reason. Boards appear here too: * a board that fails must not vanish silently, or the index quietly loses @@ -107,12 +133,23 @@ export interface Snapshot { export interface CrawlOptions { schoolId: string; + /** The account's own user id — needed to reach its personal files. */ + userId?: string; /** Restrict to these courses. Omit for everything the account can see. */ courseIds?: string[]; /** Fetch lesson bodies too. Costs one request per lesson. */ includeLessonContents?: boolean; /** Resolve board file elements to file records. */ includeFiles?: boolean; + /** + * Also index the account's personal files and its submitted / returned work. + * + * Off by default because of what it costs: submissions are per task, so this + * adds roughly three requests per task on top of a crawl that is already the + * expensive part of this server. Worth it when you want "what did I write + * about X" to be searchable, which is otherwise impossible. + */ + includePersonalFiles?: boolean; /** * Read the text of collaborative text editor (Etherpad) pads, which needs a * second credentialled hop outside the API. Omit to leave pads unread. @@ -133,14 +170,30 @@ export async function crawl(client: SchulcloudClient, options: CrawlOptions): Pr const crawled: CrawledCourse[] = []; const files: CrawledFile[] = []; + const submissions: CrawledSubmission[] = []; const failures: { courseId: string; boardId?: string; reason: string }[] = []; let done = 0; + // Personal files ("Meine Dateien") hang off the user, not off any course, so + // nothing in the course walk would ever reach them. One request, and only on + // a full crawl — a per-course refresh has no business rewriting them. + if (includeFiles && options.includePersonalFiles && !options.courseIds && options.userId) { + for (const record of await listFiles(client, options.schoolId, 'users', options.userId)) { + files.push({ + record, + parentType: 'users', + parentId: options.userId, + at: { courseId: '', courseTitle: 'My files' }, + }); + } + } + await forEachLimited(courses, options.courseConcurrency ?? 5, async (course) => { try { const result = await crawlCourse(client, course, options, includeFiles, includeLessons); crawled.push(result.course); files.push(...result.files); + submissions.push(...result.submissions); failures.push(...result.failures); } catch (error) { failures.push({ courseId: course.id, reason: error instanceof Error ? error.message : String(error) }); @@ -159,8 +212,17 @@ export async function crawl(client: SchulcloudClient, options: CrawlOptions): Pr crawled.sort((a, b) => a.course.id.localeCompare(b.course.id)); rooms.sort((a, b) => a.id.localeCompare(b.id)); files.sort((a, b) => a.record.id.localeCompare(b.record.id)); + submissions.sort((a, b) => a.id.localeCompare(b.id)); - return { crawledAt: new Date(), schoolId: options.schoolId, courses: crawled, rooms, files, failures }; + return { + crawledAt: new Date(), + schoolId: options.schoolId, + courses: crawled, + rooms, + files, + submissions, + failures, + }; } /** @@ -281,10 +343,16 @@ async function crawlCourse( options: CrawlOptions, includeFiles: boolean, includeLessons: boolean, -): Promise<{ course: CrawledCourse; files: CrawledFile[]; failures: { courseId: string; boardId: string; reason: string }[] }> { +): Promise<{ + course: CrawledCourse; + files: CrawledFile[]; + submissions: CrawledSubmission[]; + failures: { courseId: string; boardId: string; reason: string }[]; +}> { const page = await client.getCourseBoard(course.id); const title = page.title || course.title; const files: CrawledFile[] = []; + const submissions: CrawledSubmission[] = []; const failures: { courseId: string; boardId: string; reason: string }[] = []; const boards: CrawledBoard[] = []; @@ -307,6 +375,9 @@ async function crawlCourse( at: { courseId: course.id, courseTitle: title, containerTitle: element.content.name }, }); } + if (options.includePersonalFiles) { + await collectSubmissions(client, options, course.id, title, element.content, files, submissions); + } } } else if (element.type === 'lesson') { const lesson: CrawledLesson = { @@ -333,6 +404,45 @@ async function crawlCourse( } } lessons.push(lesson); + + // Tasks attached to a topic are not task elements on the course page, + // so nothing above reaches them — and once past due they are absent + // from both task lists too. On the account this was built for that is + // 18 of 60 tasks: without this they are unsearchable and their grades + // are invisible. The ids only exist on the topic page (lesson-page.ts). + if (options.config && element.content.numberOfPublishedTasks) { + const [linked, links] = await Promise.all([ + client.getLessonTasks(element.content.id).catch(() => []), + fetchLessonTaskLinks(options.config, course.id, element.content.id), + ]); + for (const linkedTask of withScrapedIds(linked, links)) { + if (!linkedTask.id) continue; + const body = htmlToText(linkedTask.description); + const asTask: TaskContent = { + id: linkedTask.id, + name: linkedTask.name, + courseId: course.id, + courseName: title, + lessonName: element.content.name, + description: linkedTask.description, + dueDate: linkedTask.dueDate ?? null, + availableDate: linkedTask.availableDate, + status: { + submitted: 0, + maxSubmissions: 0, + graded: 0, + isDraft: false, + isSubstitutionTeacher: false, + isFinished: false, + }, + }; + tasks.push({ id: linkedTask.id, courseId: course.id, task: asTask, text: body }); + if (includeFiles && options.includePersonalFiles) { + await collectSubmissions(client, options, course.id, title, asTask, files, submissions); + } + } + } + if (includeFiles) { for (const record of await listFiles(client, options.schoolId, 'lessons', element.content.id)) { files.push({ @@ -346,16 +456,94 @@ async function crawlCourse( } } + // The course's own file area ("Dateien" on the course page). One request per + // course, and previously invisible: these files were reachable with + // list_files but never indexed, so search and `schulcloud sync` missed them. + if (includeFiles) { + for (const record of await listFiles(client, options.schoolId, 'courses', course.id)) { + files.push({ + record, + parentType: 'courses', + parentId: course.id, + at: { courseId: course.id, courseTitle: title, containerTitle: 'Course files' }, + }); + } + } + await crawlBoards(client, options, { id: course.id, title }, boardIds, includeFiles, boards, files, failures); boards.sort((a, b) => a.id.localeCompare(b.id)); - return { course: { course, title, boards, lessons, tasks }, files, failures }; + return { course: { course, title, boards, lessons, tasks }, files, submissions, failures }; +} + +/** + * What was handed in for one task, and what the teacher handed back. + * + * Both hang off the *submission* id, which is only obtainable from the status + * endpoint — there is no submissions list. Note the asymmetry the file service + * has here: listing with `parentType: 'gradings'` returns records whose own + * `parentType` is `submissions`, so each record is filed under what it says it + * is rather than under the path it was asked for. Without that split a + * student's own upload would be indexed as teacher feedback. + */ +async function collectSubmissions( + client: SchulcloudClient, + options: CrawlOptions, + courseId: string, + courseTitle: string, + task: TaskContent, + files: CrawledFile[], + submissions: CrawledSubmission[], +): Promise { + const statuses = await client.listSubmissionStatuses(task.id).catch(() => []); + if (statuses.length === 0) return; + + // The grade comment and the submitted text exist only on the rendered page, + // so one fetch per task covers every submission on it. + const page = options.config + ? await fetchHomeworkPage(options.config, task.id).catch(() => undefined) + : undefined; + + for (const status of statuses) { + const grading = page?.grading.find((entry) => entry.submissionId === status.id); + submissions.push({ + id: status.id, + taskId: task.id, + taskName: task.name, + courseId, + courseTitle, + isSubmitted: status.isSubmitted, + isGraded: status.isGraded, + grade: status.grade ?? grading?.gradePercent ?? null, + gradeComment: grading?.gradeComment ?? page?.own?.gradeComment, + submittedText: page?.own?.submittedText, + submitterIds: status.submitters, + }); + } + + for (const status of statuses) { + for (const parentType of ['submissions', 'gradings'] as const) { + for (const record of await listFiles(client, options.schoolId, parentType, status.id)) { + files.push({ + record, + parentType: record.parentType ?? parentType, + parentId: status.id, + at: { + courseId, + courseTitle, + containerTitle: task.name, + cardTitle: record.parentType === 'gradings' ? 'Returned by the teacher' : 'Handed in', + }, + }); + } + } + } } async function listFiles( client: SchulcloudClient, schoolId: string, - parentType: 'lessons' | 'tasks', + parentType: FileParentType, parentId: string, ): Promise { const page = await client diff --git a/src/core/etherpad.ts b/src/core/etherpad.ts index 4b8a49b..643002c 100644 --- a/src/core/etherpad.ts +++ b/src/core/etherpad.ts @@ -98,3 +98,57 @@ export function padIdFromUrl(url: string, baseUrl: string): string | undefined { const segment = /\/etherpad\/p\/([^/?#]+)/.exec(parsed.pathname)?.[1]; return segment && segment.length > 0 ? segment : undefined; } + +/** + * The text of a pad linked from a *topic* ("Thema"), as opposed to a board. + * + * Topics reach Etherpad differently from column boards: there is no + * `collaborative-text-editor` element to ask, only a stored pad url on the + * lesson component. The session cookie comes from the topic page instead — + * the legacy client requests an Etherpad session on every topic page whose + * lesson has contents, whether or not a pad is present, so simply rendering + * the page yields one. Everything here is a GET. + * + * The stored url is data and may point anywhere; `padIdFromUrl` refuses any + * host but this instance's, so a pad recorded against another deployment is + * reported as a link rather than fetched — which is the correct outcome, not + * a failure. + */ +export async function fetchLessonPadText( + config: Config, + courseId: string, + lessonId: string, + padUrl: string, +): Promise { + const padId = padIdFromUrl(padUrl, config.baseUrl); + if (!padId) return undefined; + + try { + const page = await fetch( + `${config.baseUrl}/courses/${encodeURIComponent(courseId)}/topics/${encodeURIComponent(lessonId)}`, + { + headers: { Cookie: `jwt=${config.jwt}`, Accept: 'text/html' }, + signal: AbortSignal.timeout(config.requestTimeoutMs), + redirect: 'follow', + }, + ); + if (!page.ok) return undefined; + + const sessionCookie = page.headers + .getSetCookie() + .map((cookie) => /^(sessionID=[^;]*)/.exec(cookie)?.[1]) + .find((value): value is string => Boolean(value)); + if (!sessionCookie) return undefined; + + const response = await fetch(`${new URL(config.baseUrl).origin}/etherpad/p/${padId}/export/txt`, { + headers: { Cookie: sessionCookie, Accept: 'text/plain' }, + signal: AbortSignal.timeout(config.requestTimeoutMs), + }); + if (!response.ok) return undefined; + + const text = (await response.text()).trim(); + return text.length > 0 && text !== DEFAULT_PAD_TEXT ? text : undefined; + } catch { + return undefined; + } +} diff --git a/src/core/homework-page.ts b/src/core/homework-page.ts index b3f7845..2ddb31a 100644 --- a/src/core/homework-page.ts +++ b/src/core/homework-page.ts @@ -38,12 +38,51 @@ export interface SubmissionDetail { submittedFiles: { id: string; name: string }[]; } +/** + * One submission as the *teacher's* grading form holds it. + * + * The teacher view of `/homework/{id}` is a different page from the student's: + * its tabs are `extended` and `submissions` rather than `submission` and + * `feedback`, and the grade lives in the editable form rather than in rendered + * prose. The student parser therefore finds nothing on it, which is why a + * teacher account reported every graded submission as "neither a percentage nor + * feedback was found" while the data was plainly there. + */ +export interface SubmissionGrading { + submissionId: string; + /** Ids from the form's `teamMembers` field — who handed this in. */ + submitterIds: string[]; + gradeComment?: string; + gradePercent?: number; +} + +/** Everything one homework page yields, for whichever role is looking at it. */ +export interface HomeworkPage { + /** The account's own submission, when the page is the student view. */ + own?: SubmissionDetail; + /** Every submission on the grading form, when the page is the teacher view. */ + grading: SubmissionGrading[]; +} + +export async function fetchHomeworkPage(config: Config, taskId: string): Promise { + const html = await fetchHomeworkHtml(config, taskId); + if (html === undefined) return undefined; + const own = parseHomeworkPage(html); + const grading = parseTeacherGrading(html); + if (!own && grading.length === 0) return undefined; + return { own, grading }; +} + export async function fetchSubmissionDetail( config: Config, taskId: string, ): Promise { + const html = await fetchHomeworkHtml(config, taskId); + return html === undefined ? undefined : parseHomeworkPage(html); +} + +async function fetchHomeworkHtml(config: Config, taskId: string): Promise { const url = `${config.baseUrl}/homework/${encodeURIComponent(taskId)}`; - let html: string; try { const response = await fetch(url, { headers: { Cookie: `jwt=${config.jwt}`, Accept: 'text/html' }, @@ -55,12 +94,51 @@ export async function fetchSubmissionDetail( if (!response.ok || !new URL(response.url).hostname.endsWith(new URL(config.baseUrl).hostname)) { return undefined; } - html = await response.text(); + return await response.text(); } catch { return undefined; } +} - return parseHomeworkPage(html); +/** + * Exported for testing: reads the teacher's grading form. + * + * Anchored on the form's own `name=` attributes rather than on layout, because + * those are what the POST handler reads and so cannot drift without the feature + * itself changing. Each submission contributes one `submissionId` hidden input, + * a `teamMembers` input naming who handed it in, a `grade` number input, and a + * `gradeComment` textarea whose body is HTML-escaped twice over. + */ +export function parseTeacherGrading(html: string): SubmissionGrading[] { + const found: SubmissionGrading[] = []; + const blocks = html.split(/]*value="([^"]*)"/.exec(block)?.[1] ?? ''; + const submitterIds = members + .split(',') + .map((id) => id.trim()) + .filter((id) => /^[0-9a-f]{24}$/.test(id)); + + const entry: SubmissionGrading = { submissionId, submitterIds }; + + // `value=""` means ungraded; the placeholder is a hint, not a grade. + const gradeValue = /name="grade"[^>]*?value="(\d{1,3})"/.exec(block)?.[1]; + if (gradeValue !== undefined) entry.gradePercent = Number(gradeValue); + + const commentMarkup = new RegExp( + `]*data-parent-id="${submissionId}"[^>]*>([\\s\\S]*?)`, + ).exec(html)?.[1]; + const comment = clean(commentMarkup ? decodeEntities(commentMarkup) : undefined); + if (comment) entry.gradeComment = comment; + + found.push(entry); + } + return found; } /** Exported for testing: the parsing is pure and deserves fixtures, not a network. */ diff --git a/src/core/text.ts b/src/core/text.ts index 03c7125..c98be2c 100644 --- a/src/core/text.ts +++ b/src/core/text.ts @@ -57,8 +57,24 @@ export function htmlToText(html: string | undefined | null): string { if (!html) return ''; return decodeEntities( html + // A newline in HTML source is just whitespace; only tags make lines. + // Flattening first is what stops `
` followed by a newline — the + // shape every server-side template produces — from reading as a + // paragraph break, and it takes the template's indentation with it. + .replace(/\s*\n\s*/g, ' ') .replace(//gi, '\n') - .replace(/<\/(p|div|h[1-6]|li|tr)>/gi, '\n') + // A cell whose text is wrapped in its own

— which is what the + // editor produces — would otherwise break its row in half. + .replace(/<(td|th)([^>]*)>\s*]*>/gi, '<$1$2>') + .replace(/<\/p>\s*<\/(td|th)>/gi, '') + // Cells before rows: a table flattened without cell separators runs + // its columns together, which is how a two-column worksheet grid came + // out as a meaningless list of fragments. + .replace(/<\/(td|th)>/gi, ' | ') + // Paragraphs and headings read as paragraphs; list items and table + // rows are single lines. + .replace(/<\/(p|h[1-6])>/gi, '\n\n') + .replace(/<\/(div|li|tr)>/gi, '\n') .replace(/]*>/gi, '- ') // Keep the href when the anchor text does not already contain it. .replace(/]*href="([^"]*)"[^>]*>(.*?)<\/a>/gis, (_, href: string, label: string) => { @@ -68,7 +84,14 @@ export function htmlToText(html: string | undefined | null): string { }) .replace(/<[^>]+>/g, ''), ) - .replace(/[ \t]+\n/g, '\n') + // Source HTML is pretty-printed, so nearly every line arrives with the + // template's indentation still attached. Collapsing runs of spaces and + // trimming each line is what HTML rendering would have done anyway, and + // without it a submission reads as prose adrift in whitespace. + .replace(/[^\S\n]+/g, ' ') + .split('\n') + .map((line) => line.trim().replace(/\s*\|\s*$/, '')) + .join('\n') .replace(/\n{3,}/g, '\n\n') .trim(); } diff --git a/src/core/types.ts b/src/core/types.ts index ea534e1..ddedc9c 100644 --- a/src/core/types.ts +++ b/src/core/types.ts @@ -257,8 +257,14 @@ export interface RoomItem { endDate?: string; createdAt?: string; updatedAt?: string; - /** What this account may do here — 'room_edit_content' and friends. */ - allowedOperations?: string[]; + /** + * What this account may do here. + * + * An object keyed by operation, not a list of granted ones: every operation + * is present and false means denied. Typing it as `string[]` type-checked + * fine and threw `.some is not a function` the moment anything read it. + */ + allowedOperations?: Record; isLocked?: boolean; totalMembers?: number; } @@ -294,6 +300,25 @@ export interface RoomMember { schoolName?: string; } +/** Someone who has asked to join a room and is waiting for an admin. */ +export interface RoomApplicant { + userId?: string; + firstName?: string; + lastName?: string; + schoolName?: string; + requestedAt?: string; +} + +/** A shareable link into a room. Visible only to accounts that may manage them. */ +export interface RoomInvitationLink { + id: string; + title?: string; + activeUntil?: string; + isOnlyForTeachers?: boolean; + restrictedToCreatorSchool?: boolean; + requiresConfirmation?: boolean; +} + export const FILE_PARENT_TYPES: FileParentType[] = [ 'users', 'schools', @@ -349,3 +374,84 @@ export interface NewsResponse { creator?: { id: string; firstName?: string; lastName?: string }; createdAt?: string; } + +/** + * A course as the legacy `/api/v1/courses` service returns it. + * + * The v3 projection (`CourseMetadataResponse`) carries only id, title, colour + * and dates — no description, no teachers, no members, no timetable. All of + * that still exists, and `/api/v1/courses` is one of exactly three legacy + * routes the deployment's own ingress table still publishes + * (`dof_app_deploy/ansible/group_vars/all/x_ingress.yml`: courses, users, + * classes), so this is production surface rather than a leftover. + */ +export interface LegacyCourse { + _id?: string; + id?: string; + name?: string; + description?: string; + color?: string; + startDate?: string; + untilDate?: string; + isArchived?: boolean; + teacherIds?: string[]; + substitutionIds?: string[]; + userIds?: string[]; + classIds?: string[]; + /** The weekly timetable: one entry per recurring slot. */ + times?: CourseTime[]; +} + +/** One recurring slot of a course's weekly timetable. */ +export interface CourseTime { + /** 0 = Monday, as the legacy client renders it. */ + weekday?: number; + /** Milliseconds since midnight. */ + startTime?: number; + /** Milliseconds. */ + duration?: number; + room?: string; +} + +/** A user as `/api/v1/users/{id}` returns it — the only way to turn an id into a name. */ +export interface LegacyUser { + _id?: string; + id?: string; + firstName?: string; + lastName?: string; + fullName?: string; + displayName?: string; +} + +/** A class ("Klasse") from `/api/v3/groups/class`. */ +export interface ClassItem { + id: string; + name?: string; + type?: string; + teacherNames?: string[]; + studentCount?: number; + isUpgradable?: boolean; +} + +/** A group from `/api/v3/groups` — room membership groups, classes, courses. */ +export interface GroupItem { + id: string; + name?: string; + type?: string; + organizationId?: string; + users?: { id: string; firstName?: string; lastName?: string; role?: string }[]; +} + +/** `GET /api/v3/file/stats/{parentType}/{parentId}`. */ +export interface ParentFileStats { + fileCount: number; + totalSizeInBytes: number; +} + +/** + * Widths the preview endpoint accepts. + * + * An enum rather than a free number — `width=1600` is rejected as a validation + * error that names the value but not the permitted set. + */ +export type PreviewWidth = 50 | 150 | 500; diff --git a/src/indexer/indexer.ts b/src/indexer/indexer.ts index 1433796..eec0610 100644 --- a/src/indexer/indexer.ts +++ b/src/indexer/indexer.ts @@ -109,12 +109,14 @@ export class Indexer { private async run(scope: string): Promise { const began = Date.now(); try { - const schoolId = (await this.client.me()).school.id; + const me = await this.client.me(); const snapshot: Snapshot = await crawl(this.client, { - schoolId, + schoolId: me.school.id, + userId: me.user.id, courseIds: scope === 'full' ? undefined : [scope], includeLessonContents: true, includeFiles: true, + includePersonalFiles: this.config.indexPersonalFiles, config: this.config, }); diff --git a/src/mcp/tools/content.ts b/src/mcp/tools/content.ts index 3d2a892..b09f30c 100644 --- a/src/mcp/tools/content.ts +++ b/src/mcp/tools/content.ts @@ -6,10 +6,13 @@ import { formatBytes } from '../../core/extract.ts'; import { dueLabel, formatDate, heading, htmlToText, joinSections, normalizeObjectId } from '../../core/text.ts'; import { assembleBoard, type AssembledBoard, type AssembledElement } from '../../core/board.ts'; import { forEachLimited } from '../../core/crawl.ts'; +import { fetchLessonPadText } from '../../core/etherpad.ts'; import { fetchLessonTaskLinks, withScrapedIds } from '../../core/lesson-page.ts'; import type { CourseBoardResponse, + CourseTime, FileRecord, + LegacyCourse, LessonLinkedTask, LessonResponse, ResolvedTask, @@ -35,8 +38,17 @@ export function registerContentTools(server: McpServer, context: ServerContext): }, async ({ courseId }) => { try { - const board = await context.client.getCourseBoard(courseId); - return text(formatCourseBoard(board)); + const [board, legacy] = await Promise.all([ + context.client.getCourseBoard(courseId), + // The v3 projection carries no description, teachers, members or + // timetable; /api/v1/courses still does. Optional on purpose — it + // is a legacy route, so its absence must cost detail, not the call. + context.client.getLegacyCourse(courseId).catch(() => undefined), + ]); + const teachers = legacy + ? await context.resolveNames([...(legacy.teacherIds ?? []), ...(legacy.substitutionIds ?? [])]) + : { names: [], unresolved: 0 }; + return text(formatCourseBoard(board, legacy, teachers)); } catch (error) { return toToolError(error, `read course ${courseId}`); } @@ -114,7 +126,19 @@ export function registerContentTools(server: McpServer, context: ServerContext): tasks.length > 0 ? withScrapedIds(tasks, await fetchLessonTaskLinks(context.config, lesson.courseId, lessonId)) : tasks; - return text(formatLesson(lesson, withIds, files?.data ?? [])); + // Pads are fetched only when the topic actually has one: each costs a + // topic-page render to obtain the Etherpad session cookie. + const padTexts = new Map(); + await Promise.all( + (lesson.contents ?? []).map(async (entry, index) => { + if (entry.component !== 'Etherpad') return; + const url = entry.content?.url; + if (typeof url !== 'string') return; + const padText = await fetchLessonPadText(context.config, lesson.courseId, lessonId, url); + if (padText) padTexts.set(index, padText); + }), + ); + return text(formatLesson(lesson, withIds, files?.data ?? [], padTexts)); } catch (error) { return toToolError(error, `read lesson ${lessonId}`); } @@ -238,7 +262,11 @@ async function taskFromCourse( // --- formatting -------------------------------------------------------- -function formatCourseBoard(board: CourseBoardResponse): string { +function formatCourseBoard( + board: CourseBoardResponse, + legacy?: LegacyCourse, + teachers: { names: string[]; unresolved: number } = { names: [], unresolved: 0 }, +): string { const boards: string[] = []; const lessons: string[] = []; const tasks: string[] = []; @@ -258,19 +286,72 @@ function formatCourseBoard(board: CourseBoardResponse): string { } } + const about = joinSections([ + htmlToText(legacy?.description)?.trim() || undefined, + formatTeachers(teachers), + legacy?.userIds?.length ? `**Members:** ${legacy.userIds.length}` : undefined, + formatCourseTimes(legacy?.times), + ]); + if (boards.length + lessons.length + tasks.length === 0) { - return `${heading(2, board.title)}\n\nThis course page is empty.`; + return joinSections([heading(2, board.title), about, 'This course page is empty.']); } return joinSections([ heading(2, board.title), `Course id: \`${board.roomId}\``, + about, boards.length > 0 && joinSections([heading(3, `Boards (${boards.length})`), boards.join('\n'), 'Read one with get_board.']), lessons.length > 0 && joinSections([heading(3, `Topics (${lessons.length})`), lessons.join('\n'), 'Read one with get_lesson.']), tasks.length > 0 && joinSections([heading(3, `Tasks (${tasks.length})`), tasks.join('\n'), 'Read one with get_task.']), ]); } +/** + * Who teaches the course. + * + * A student may not read their teachers' user records, so names are often + * unavailable; say how many there are rather than printing bare ids, which + * are no use to a reader and look like a bug. + */ +function formatTeachers(teachers: { names: string[]; unresolved: number }): string | undefined { + const { names, unresolved } = teachers; + if (names.length === 0 && unresolved === 0) return undefined; + if (names.length === 0) { + return `**Taught by:** ${unresolved} teacher(s) — names are not visible to this account`; + } + const rest = unresolved > 0 ? ` and ${unresolved} more (name not visible to this account)` : ''; + return `**Taught by:** ${names.join(', ')}${rest}`; +} + +/** + * A course's weekly timetable. + * + * `times` is the closest thing to a calendar the API exposes — the calendar + * service itself is not part of the v3 document. `startTime` is milliseconds + * since midnight and `weekday` is 0-based from Monday, as the legacy client + * renders it. + */ +function formatCourseTimes(times: CourseTime[] | undefined): string | undefined { + if (!times || times.length === 0) return undefined; + const days = ['Monday', 'Tuesday', 'Wednesday', 'Thursday', 'Friday', 'Saturday', 'Sunday']; + const rows = times + .slice() + .sort((a, b) => (a.weekday ?? 0) - (b.weekday ?? 0) || (a.startTime ?? 0) - (b.startTime ?? 0)) + .map((slot) => { + const day = days[slot.weekday ?? 0] ?? `day ${slot.weekday}`; + const room = slot.room ? `, room ${slot.room}` : ''; + return `- ${day} ${clockFromMs(slot.startTime)}${slot.duration ? `–${clockFromMs((slot.startTime ?? 0) + slot.duration)}` : ''}${room}`; + }); + return joinSections([`**Weekly schedule:**`, rows.join('\n')]); +} + +function clockFromMs(ms: number | undefined): string { + if (ms === undefined) return '?'; + const total = Math.floor(ms / 60_000); + return `${String(Math.floor(total / 60)).padStart(2, '0')}:${String(total % 60).padStart(2, '0')}`; +} + function formatBoard(board: AssembledBoard, includeFiles: boolean): string { const columns = board.columns.map((column) => { const cards = column.cards.map((card) => { @@ -312,16 +393,24 @@ function formatElement(element: AssembledElement, includeFiles: boolean): string } case 'link': { const label = element.text?.trim(); - return element.url ? `- Link: ${label && label !== element.url ? `${label} — ${element.url}` : element.url}` : ''; + if (!element.url) return ''; + const head = `- Link: ${label && label !== element.url ? `${label} — ${element.url}` : element.url}`; + const note = htmlToText(element.description)?.trim(); + return note ? `${head}\n${indent(note)}` : head; } case 'file': case 'fileFolder': case 'drawing': { const caption = element.text ? ` — caption: ${element.text}` : ''; - if (!includeFiles) return `- ${element.type} element \`${element.id}\`${caption}`; + // Alt text describes the picture itself, so it belongs on the line + // whether or not the file records could be listed. + const alt = element.alternativeText?.trim() ? ` — alt: ${element.alternativeText.trim()}` : ''; + const note = element.description?.trim() ? ` — ${element.description.trim()}` : ''; + const extra = `${caption}${alt}${note}`; + if (!includeFiles) return `- ${element.type} element \`${element.id}\`${extra}`; if (element.fileError) return `- ${element.type} element \`${element.id}\` — could not list files (${element.fileError})`; - if (element.files.length === 0) return `- ${element.type} element \`${element.id}\` — no files${caption}`; - return element.files.map((file) => `- ${formatFileLine(file)}${caption}`).join('\n'); + if (element.files.length === 0) return `- ${element.type} element \`${element.id}\` — no files${extra}`; + return element.files.map((file) => `- ${formatFileLine(file)}${extra}`).join('\n'); } case 'collaborativeTextEditor': { const title = element.text ? ` — ${element.text}` : ''; @@ -333,14 +422,29 @@ function formatElement(element: AssembledElement, includeFiles: boolean): string } return [`- Collaborative text document \`${element.id}\`${title}:`, indent(element.padText)].join('\n'); } - case 'externalTool': - return `- External tool${element.text ? `: ${element.text}` : ''} \`${element.id}\``; + case 'externalTool': { + // The configured-tool id is what `api_get /api/v3/tools/...` needs to + // say which tool this actually is; without it the element is opaque. + const tool = element.contextExternalToolId + ? ` — configured tool \`${element.contextExternalToolId}\`` + : ''; + return `- External tool${element.text ? `: ${element.text}` : ''} \`${element.id}\`${tool}`; + } case 'videoConference': - return `- Video conference \`${element.id}\``; - case 'h5p': - return `- H5P interactive content \`${element.id}\``; - case 'deleted': - return '- _(deleted element)_'; + return `- Video conference${element.text ? `: ${element.text}` : ''} \`${element.id}\``; + case 'h5p': { + // Schulcloud has no quiz of its own: interactive exercises are H5P, and + // this id is the only way to reach the content behind one. + const content = element.h5pContentId ? ` — H5P content \`${element.h5pContentId}\`` : ''; + return `- H5P interactive content \`${element.id}\`${content}`; + } + case 'deleted': { + // Saying what it was beats "(deleted element)": the title often names + // the material a student is looking for and cannot find. + const was = element.deletedElementType ? ` ${element.deletedElementType}` : ''; + const title = element.text ? `: ${element.text}` : ''; + return `- _(deleted${was} element${title})_`; + } default: return `- ${element.type} element \`${element.id}\``; } @@ -352,12 +456,17 @@ export function formatFileLine(file: FileRecord): string { return `File: **${file.name}** (\`${file.id}\`, ${file.mimeType}, ${formatBytes(file.size)})${blocked}${pending}`; } -function formatLesson(lesson: LessonResponse, tasks: LessonLinkedTask[], files: FileRecord[]): string { - const sections = (lesson.contents ?? []).map((entry) => { +function formatLesson( + lesson: LessonResponse, + tasks: LessonLinkedTask[], + files: FileRecord[], + padTexts: Map = new Map(), +): string { + const sections = (lesson.contents ?? []).map((entry, index) => { const title = entry.title?.trim(); const component = entry.component ?? 'unknown'; const hidden = entry.hidden ? ' [hidden]' : ''; - const body = formatLessonComponent(component, entry.content ?? {}); + const body = formatLessonComponent(component, entry.content ?? {}, padTexts.get(index)); return joinSections([heading(4, `${title || component}${hidden}`), body || `_(${component} content, nothing to show)_`]); }); @@ -386,16 +495,40 @@ function formatLesson(lesson: LessonResponse, tasks: LessonLinkedTask[], files: ]); } -function formatLessonComponent(component: string, content: Record): string { +function formatLessonComponent( + component: string, + content: Record, + padText?: string, +): string { if (component === 'text' && typeof content.text === 'string') return htmlToText(content.text); if (component === 'resources' && Array.isArray(content.resources)) { return content.resources .map((resource) => { const entry = resource as { title?: string; url?: string; description?: string }; - return `- ${entry.title ?? 'Resource'}${entry.url ? ` — ${entry.url}` : ''}`; + const note = entry.description?.trim(); + const head = `- ${entry.title ?? 'Resource'}${entry.url ? ` — ${entry.url}` : ''}`; + return note ? `${head}\n${indent(note)}` : head; }) .join('\n'); } + // A topic's Etherpad: the same collaborative document a column board can + // hold, reached by a stored url instead of an element id. The url is data + // and may name another deployment, in which case the text is unavailable + // and the link is the honest answer. + if (component === 'Etherpad') { + const url = typeof content.url === 'string' ? content.url : undefined; + const note = typeof content.description === 'string' ? content.description.trim() : ''; + const lines = [url ? `- Collaborative text document — ${url}` : '- Collaborative text document']; + if (note) lines.push(indent(note)); + if (padText) lines.push(indent(padText)); + else if (url) lines.push(indent('_(empty, or its contents could not be read)_')); + return lines.join('\n'); + } + // A GeoGebra applet. Only the material id is stored, so name it and let the + // reader follow it rather than rendering an empty section. + if (component === 'geoGebra' && typeof content.materialId === 'string') { + return `- GeoGebra applet \`${content.materialId}\` — https://www.geogebra.org/m/${content.materialId}`; + } if (typeof content.url === 'string') return `- ${content.url}`; if (typeof content.title === 'string') return content.title; return ''; diff --git a/src/mcp/tools/files.ts b/src/mcp/tools/files.ts index dd0fb7e..0880aab 100644 --- a/src/mcp/tools/files.ts +++ b/src/mcp/tools/files.ts @@ -30,11 +30,22 @@ export function registerFileTools(server: McpServer, context: ServerContext): vo async ({ parentType, parentId }) => { try { const schoolId = await context.schoolId(); - const page = await context.client.listFiles({ storageLocationId: schoolId, parentType, parentId }); + const [page, stats] = await Promise.all([ + context.client.listFiles({ storageLocationId: schoolId, parentType, parentId }), + // Cheap, and it is the only way to see that a parent holds files + // the listing paged past. + context.client.getParentFileStats(parentType, parentId).catch(() => undefined), + ]); if (page.data.length === 0) return text(`No files attached to ${parentType} ${parentId}.`); + const total = + stats && stats.fileCount > page.data.length + ? ` — ${stats.fileCount} in total, ${formatBytes(stats.totalSizeInBytes)}` + : stats + ? ` — ${formatBytes(stats.totalSizeInBytes)} in total` + : ''; return text( joinSections([ - heading(2, `Files on ${parentType} ${parentId} (${page.data.length})`), + heading(2, `Files on ${parentType} ${parentId} (${page.data.length})${total}`), page.data.map((file) => `- ${formatFileLine(file)} — uploaded ${formatDate(file.createdAt)}`).join('\n'), 'Read one with download_file.', ]), @@ -80,7 +91,12 @@ export function registerFileTools(server: McpServer, context: ServerContext): vo `"${record.name}" was blocked by the instance's virus scanner and will not be downloaded.`, ); } - const file = await context.client.downloadFile(record); + const [file, uploader] = await Promise.all([ + context.client.downloadFile(record), + // Who put the file there is often the quickest way to tell a + // teacher's material apart from a classmate's upload. + record.creatorId ? context.userName(record.creatorId) : Promise.resolve(undefined), + ]); const header = [ heading(2, record.name), [ @@ -88,7 +104,7 @@ export function registerFileTools(server: McpServer, context: ServerContext): vo `- Type: ${record.mimeType}`, `- Size: ${formatBytes(record.size)}`, `- Attached to: ${record.parentType} \`${record.parentId}\``, - `- Uploaded: ${formatDate(record.createdAt)}`, + `- Uploaded: ${formatDate(record.createdAt)}${uploader ? ` by ${uploader}` : ''}`, record.securityCheckStatus !== 'verified' ? `- Virus scan: ${record.securityCheckStatus}` : undefined, @@ -140,6 +156,32 @@ export function registerFileTools(server: McpServer, context: ServerContext): vo ); } + // Nothing extractable — but files-storage may still be able to render + // the file as a picture. That is the whole answer for an image-only + // PDF: its pages *are* pictures, so a rasterised preview is readable + // where the bytes are not, and it needs no OCR on our side. + if (record.previewStatus === 'preview_possible') { + const preview = await context.client.getFilePreview(record, 500).catch(() => undefined); + if (preview && preview.mimeType.startsWith('image/')) { + const result: CallToolResult = { + content: [ + { + type: 'text', + text: joinSections([ + header, + // The note ends by suggesting raw bytes, which is no longer the + // best answer once a readable rendering is attached. + extraction.note.replace(' Use download_file with raw=true to get the bytes.', ''), + "Showing the instance's own rendered preview below, which is readable as a picture.", + ]), + }, + { type: 'image', data: preview.bytes.toString('base64'), mimeType: preview.mimeType }, + ], + }; + return result; + } + } + return text(joinSections([header, extraction.note])); } catch (error) { return toToolError(error, `download file ${fileId}`); diff --git a/src/mcp/tools/rooms.ts b/src/mcp/tools/rooms.ts index 167012b..b22af41 100644 --- a/src/mcp/tools/rooms.ts +++ b/src/mcp/tools/rooms.ts @@ -2,7 +2,13 @@ import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; import { z } from 'zod'; import type { ServerContext } from '../../context.ts'; import { formatDate, heading, joinSections } from '../../core/text.ts'; -import type { RoomBoardItem, RoomMember } from '../../core/types.ts'; +import type { + RoomApplicant, + RoomBoardItem, + RoomDetails, + RoomInvitationLink, + RoomMember, +} from '../../core/types.ts'; import { text, toToolError } from './result.ts'; const READ_ONLY = { readOnlyHint: true, destructiveHint: false, idempotentHint: true, openWorldHint: true }; @@ -76,20 +82,99 @@ export function registerRoomTools(server: McpServer, context: ServerContext): vo try { // Members and boards are both allowed to fail without costing the room: // a viewer may be refused the member list, and boards can be empty. - const [room, boards, members] = await Promise.all([ - context.client.getRoom(roomId), + const room = await context.client.getRoom(roomId); + // Applicants and invitation links are room-admin surface. Ask only + // when this account is allowed to, so a viewer does not pay for two + // requests that can only come back 403. + const may = room.allowedOperations ?? {}; + const [boards, members, applicants, links] = await Promise.all([ context.client.listRoomBoards(roomId).catch(() => [] as RoomBoardItem[]), context.client.listRoomMembers(roomId).catch(() => [] as RoomMember[]), + may.manageRoomApplicants + ? context.client.listRoomApplicants(roomId).catch(() => [] as RoomApplicant[]) + : Promise.resolve([] as RoomApplicant[]), + may.listRoomInvitationLinks + ? context.client.listRoomInvitationLinks(roomId).catch(() => [] as RoomInvitationLink[]) + : Promise.resolve([] as RoomInvitationLink[]), ]); - return text(formatRoom(room.name, roomId, boards, members)); + return text(formatRoom(room, roomId, boards, members, applicants, links)); } catch (error) { return toToolError(error, `read room ${roomId}`); } }, ); + + server.registerTool( + 'list_classes', + { + title: 'List my classes and groups', + description: + 'The classes ("Klassen") this account belongs to, with their teachers and size, and any other ' + + 'groups it is a member of. Use it for "who is my class teacher", "which class am I in", or to ' + + 'find the people behind a course. This is the only place class membership is visible — courses ' + + 'and rooms do not report it.', + inputSchema: { + includeGroups: z + .boolean() + .default(false) + .describe('Also list non-class groups, such as the membership group behind each room.'), + }, + annotations: READ_ONLY, + }, + async ({ includeGroups }) => { + try { + const [classes, groups] = await Promise.all([ + context.client.listClasses(), + includeGroups ? context.client.listGroups().catch(() => []) : Promise.resolve([]), + ]); + if (classes.length === 0 && groups.length === 0) { + return text('This account is not in any class or group.'); + } + + const classLines = classes.map((entry) => { + const teachers = entry.teacherNames?.length ? ` — taught by ${entry.teacherNames.join(', ')}` : ''; + const size = entry.studentCount === undefined ? '' : ` — ${entry.studentCount} student(s)`; + return `- **${entry.name ?? 'Unnamed class'}** (\`${entry.id}\`)${teachers}${size}`; + }); + + // Room membership groups carry names and room roles, which is a second + // route to "who is in this room" when the members endpoint refuses. + const groupLines = groups + .filter((group) => group.type !== 'class') + .map((group) => { + const people = (group.users ?? []) + .map((user) => `${[user.firstName, user.lastName].filter(Boolean).join(' ')}${user.role ? ` (${user.role.replace(/^room/, '')})` : ''}`) + .filter(Boolean); + const who = people.length > 0 ? `\n${people.map((line) => ` - ${line}`).join('\n')}` : ''; + return `- **${group.name ?? 'Unnamed group'}** (\`${group.id}\`, ${group.type ?? 'group'})${who}`; + }); + + return text( + joinSections([ + classLines.length > 0 + ? joinSections([heading(2, `Classes (${classLines.length})`), classLines.join('\n')]) + : undefined, + groupLines.length > 0 + ? joinSections([heading(2, `Groups (${groupLines.length})`), groupLines.join('\n')]) + : undefined, + ]), + ); + } catch (error) { + return toToolError(error, 'list classes'); + } + }, + ); } -function formatRoom(name: string, roomId: string, boards: RoomBoardItem[], members: RoomMember[]): string { +function formatRoom( + room: RoomDetails, + roomId: string, + boards: RoomBoardItem[], + members: RoomMember[], + applicants: RoomApplicant[] = [], + links: RoomInvitationLink[] = [], +): string { + const name = room.name; // Room boards report `isVisible`, so an unpublished one can be named as such // instead of being offered and then answering 403 — which is all a course // board can do, since the course projection omits the flag. @@ -103,12 +188,54 @@ function formatRoom(name: string, roomId: string, boards: RoomBoardItem[], membe return `- ${who}${member.roomRoleName ? ` — ${member.roomRoleName.replace(/^room/, '')}` : ''}`; }); + // What this account may do here, and which optional features the room has + // switched on. Both are already in the response and were simply dropped — + // `allowedOperations` in particular is the difference between "you are a + // viewer" and "you could edit this", which changes what to suggest next. + const granted = Object.entries(room.allowedOperations ?? {}) + .filter(([, allowed]) => allowed) + .map(([operation]) => operation); + const canEdit = room.allowedOperations?.editContent === true; + const facts = [ + granted.length > 0 + ? `- Your access: ${canEdit ? 'can edit content' : 'read-only'} (${granted.join(', ')})` + : undefined, + room.features?.length ? `- Features: ${room.features.join(', ')}` : undefined, + room.startDate || room.endDate + ? `- Active: ${formatDate(room.startDate).slice(0, 10)} to ${formatDate(room.endDate).slice(0, 10)}` + : undefined, + ].filter(Boolean) as string[]; + return joinSections([ heading(2, name), `Room id: \`${roomId}\``, + facts.length > 0 ? facts.join('\n') : undefined, boardLines.length > 0 ? joinSections([heading(3, `Boards (${boardLines.length})`), boardLines.join('\n'), 'Read one with get_board.']) : '_No boards in this room._', memberLines.length > 0 ? joinSections([heading(3, `Members (${memberLines.length})`), memberLines.join('\n')]) : undefined, + applicants.length > 0 + ? joinSections([ + heading(3, `Waiting to join (${applicants.length})`), + applicants + .map((person) => { + const who = [person.firstName, person.lastName].filter(Boolean).join(' ') || person.userId || 'Someone'; + return `- ${who}${person.schoolName ? ` — ${person.schoolName}` : ''}`; + }) + .join('\n'), + ]) + : undefined, + links.length > 0 + ? joinSections([ + heading(3, `Invitation links (${links.length})`), + links + .map((link) => { + const until = link.activeUntil ? ` — until ${formatDate(link.activeUntil)}` : ''; + const who = link.isOnlyForTeachers ? ' — teachers only' : ''; + return `- ${link.title ?? 'Untitled link'} (\`${link.id}\`)${until}${who}`; + }) + .join('\n'), + ]) + : undefined, ]); } diff --git a/src/mcp/tools/search.ts b/src/mcp/tools/search.ts index a4cb752..d08e69d 100644 --- a/src/mcp/tools/search.ts +++ b/src/mcp/tools/search.ts @@ -15,6 +15,9 @@ const TOOL_FOR: Record = { lesson: 'get_lesson', task: 'get_task', file: 'download_file', + // A submission is reached through its task, not by an id of its own: there + // is no get_submission because the API has no route to one. + submission: 'get_task', }; export function registerSearchTool(server: McpServer, context: ServerContext): void { @@ -133,11 +136,22 @@ async function liveSearch( ]); } +function targetIdFor(hit: SearchResult): string { + if (hit.kind === 'submission') { + const taskId = hit.meta?.taskId; + if (typeof taskId === 'string') return taskId; + } + return hit.nodeId; +} + function formatIndexed(hit: SearchResult): string { return [ `- **${hit.title}** — ${hit.kind} in ${hit.courseTitle || hit.path}`, hit.snippet && hit.snippet !== hit.title ? ` ${hit.snippet}` : undefined, - ` → \`${TOOL_FOR[hit.kind] ?? 'api_get'}\` with id \`${hit.nodeId}\``, + // A submission has no id of its own that any tool takes: get_task is + // reached through the *task*, so point at that rather than at the + // submission id, which would simply 404. + ` → \`${TOOL_FOR[hit.kind] ?? 'api_get'}\` with id \`${targetIdFor(hit)}\``, ] .filter(Boolean) .join('\n'); diff --git a/src/mcp/tools/submissions.ts b/src/mcp/tools/submissions.ts index 9657838..797a310 100644 --- a/src/mcp/tools/submissions.ts +++ b/src/mcp/tools/submissions.ts @@ -3,7 +3,7 @@ import { z } from 'zod'; import type { ServerContext } from '../../context.ts'; import { fetchLessonTaskLinks, withScrapedIds } from '../../core/lesson-page.ts'; import { forEachLimited } from '../../core/crawl.ts'; -import { fetchSubmissionDetail } from '../../core/homework-page.ts'; +import { fetchHomeworkPage, fetchSubmissionDetail, type SubmissionGrading } from '../../core/homework-page.ts'; import { dueLabel, heading, joinSections } from '../../core/text.ts'; import type { FileRecord, ResolvedTask, SubmissionStatus } from '../../core/types.ts'; import { formatFileLine } from './content.ts'; @@ -44,24 +44,55 @@ export function registerSubmissionTools(server: McpServer, context: ServerContex .boolean() .default(true) .describe('Keep only submissions the account itself is a submitter on. Matters on teacher accounts.'), + includeFeedback: z + .boolean() + .default(false) + .describe( + 'Also read each task\'s page to report the written feedback and who submitted. Answers ' + + '"what did the teacher say" in one call, but costs one extra page fetch per task — ' + + 'pair it with courseId or a small limit.', + ), limit: z.number().int().min(1).max(99).default(50).describe('Maximum tasks to check.'), }, annotations: READ_ONLY, }, - async ({ courseId, scope, onlyMine, limit }) => { + async ({ courseId, scope, onlyMine, includeFeedback, limit }) => { try { const [me, tasks] = await Promise.all([context.me(), collectTasks(context, scope, courseId, limit)]); if (tasks.length === 0) return text('No tasks found to check for submissions.'); - const rows: { task: ResolvedTask; status: SubmissionStatus }[] = []; + const rows: { task: ResolvedTask; status: SubmissionStatus; grading?: SubmissionGrading }[] = []; const unavailable: string[] = []; await forEachLimited(tasks, 5, async (task) => { try { const statuses = await context.client.listSubmissionStatuses(task.id); - for (const status of statuses) { - if (onlyMine && !status.submitters.includes(me.user.id)) continue; - rows.push({ task, status }); + const kept = statuses.filter((status) => !onlyMine || status.submitters.includes(me.user.id)); + if (kept.length === 0) return; + + // Only pay for the page when asked: it is one fetch per task, and + // the status endpoint alone cannot tell feedback-only grading from + // an unmarked grade. + let grading: SubmissionGrading[] = []; + if (includeFeedback) { + const page = await fetchHomeworkPage(context.config, task.id).catch(() => undefined); + grading = page?.grading ?? []; + // The student view has no grading form; its single submission's + // feedback is still worth folding in under the same shape. + if (grading.length === 0 && page?.own && kept.length === 1 && kept[0]) { + grading = [ + { + submissionId: kept[0].id, + submitterIds: kept[0].submitters, + gradeComment: page.own.gradeComment, + gradePercent: page.own.gradePercent, + }, + ]; + } + } + + for (const status of kept) { + rows.push({ task, status, grading: grading.find((g) => g.submissionId === status.id) }); } } catch { // A task whose submissions we cannot read is worth noting, not fatal. @@ -79,10 +110,23 @@ export function registerSubmissionTools(server: McpServer, context: ServerContex } rows.sort((a, b) => Number(a.status.isGraded) - Number(b.status.isGraded)); + + // Resolve every submitter once. Without this a teacher's list is a + // wall of identical rows: the same task name repeated per student + // with nothing to tell them apart. + const names = new Map(); + await Promise.all( + [...new Set(rows.flatMap((row) => row.status.submitters))].map(async (id) => { + const name = await context.userName(id); + if (name) names.set(id, name); + }), + ); + + const formatted = await Promise.all(rows.map((row) => formatRow(row, me.user.id, names))); return text( joinSections([ heading(2, `Submissions (${rows.length} across ${tasks.length} task(s))`), - rows.map(formatRow).join('\n'), + formatted.join('\n'), 'Use get_task with a task id to see the submitted files and download them.', unavailable.length > 0 ? `_Could not check ${unavailable.length} task(s)._` : undefined, ]), @@ -126,18 +170,51 @@ export function formatGradeState( return 'marked graded, but neither a percentage nor feedback was found'; } -function formatRow({ task, status }: { task: ResolvedTask; status: SubmissionStatus }): string { +function formatRow( + { task, status, grading }: { task: ResolvedTask; status: SubmissionStatus; grading?: SubmissionGrading }, + myUserId: string, + names: Map, +): string { const state = status.isSubmitted ? 'submitted' : 'not submitted'; - // The list does not fetch pages, so it cannot know whether feedback exists; - // it says only what the API told it. - const graded = status.isGraded - ? status.grade !== null && status.grade !== undefined - ? `graded ${status.grade}%` - : 'graded (percentage not set — check get_task for written feedback)' - : 'not graded'; + + // With the page read, feedback-only grading can be named for what it is + // rather than deferred to get_task. Without it, say only what the API said. + const percent = status.grade ?? grading?.gradePercent; + const hasFeedback = Boolean(grading?.gradeComment); + let graded: string; + if (!status.isGraded) { + graded = 'not graded'; + } else if (percent !== null && percent !== undefined) { + graded = hasFeedback ? `graded ${percent}%, with feedback` : `graded ${percent}%`; + } else if (hasFeedback) { + graded = 'graded by feedback, with no percentage given'; + } else if (grading) { + graded = 'marked graded, but neither a percentage nor feedback was found'; + } else { + graded = 'graded (percentage not set — pass includeFeedback for the written feedback)'; + } + + // Who handed it in. Omitted when the caller is the only submitter, which is + // the student case and would just be noise. + const others = status.submitters.filter((id) => id !== myUserId); + const by = + others.length > 0 + ? ` — by ${status.submitters.map((id) => (id === myUserId ? 'you' : (names.get(id) ?? id))).join(', ')}` + : ''; + const group = status.submittingCourseGroupName ? ` — group "${status.submittingCourseGroupName}"` : ''; const course = task.courseName ? ` [${task.courseName}]` : ''; - return `- **${task.name}**${course} — ${state}, ${graded}${group}\n task \`${task.id}\`, submission \`${status.id}\``; + const feedback = grading?.gradeComment ? `\n feedback: ${oneLine(grading.gradeComment)}` : ''; + return ( + `- **${task.name}**${course} — ${state}, ${graded}${by}${group}` + + `\n task \`${task.id}\`, submission \`${status.id}\`${feedback}` + ); +} + +/** Feedback is prose; keep a list row a row. */ +function oneLine(value: string): string { + const collapsed = value.replace(/\s+/g, ' ').trim(); + return collapsed.length > 200 ? `${collapsed.slice(0, 197)}…` : collapsed; } async function collectTasks( @@ -194,30 +271,44 @@ export async function describeSubmission( context: ServerContext, taskId: string, ): Promise { - const [me, statuses, detail] = await Promise.all([ + const [me, statuses, page] = await Promise.all([ context.me(), context.client.listSubmissionStatuses(taskId).catch(() => [] as SubmissionStatus[]), // Submitted text and written feedback exist only in the rendered web page; // see core/homework-page.ts. Optional by construction — a failure here // costs detail, not the whole answer. - fetchSubmissionDetail(context.config, taskId).catch(() => undefined), + fetchHomeworkPage(context.config, taskId).catch(() => undefined), ]); if (statuses.length === 0) return undefined; + const detail = page?.own; const mine = statuses.filter((status) => status.submitters.includes(me.user.id)); const relevant = mine.length > 0 ? mine : statuses; const parts: string[] = []; for (const status of relevant) { const files = await loadSubmissionFiles(context, status.id); - const graded = formatGradeState(status, Boolean(detail?.gradeComment), detail?.gradePercent); + // On a teacher account the grade lives in the grading form, one entry per + // submission, rather than in the student's rendered feedback tab. + const grading = page?.grading.find((entry) => entry.submissionId === status.id); + const graded = formatGradeState( + status, + Boolean(detail?.gradeComment ?? grading?.gradeComment), + detail?.gradePercent ?? grading?.gradePercent, + ); + const submitterNames = await context.userNamesFor( + status.submitters.filter((id) => id !== me.user.id), + ); parts.push( [ - `- Submission \`${status.id}\`${mine.length === 0 ? ' _(not yours)_' : ''}`, + `- Submission \`${status.id}\`${status.submitters.includes(me.user.id) ? '' : ' _(not yours)_'}`, + submitterNames.length > 0 ? `- Handed in by: ${submitterNames.join(', ')}` : undefined, `- ${status.isSubmitted ? 'Submitted' : 'Not submitted'}, ${graded}`, status.submittingCourseGroupName ? `- Group: ${status.submittingCourseGroupName}` : undefined, - status.submitters.length > 1 ? `- ${status.submitters.length} submitters` : undefined, + grading?.gradeComment && !detail?.gradeComment + ? `- Feedback: ${grading.gradeComment}` + : undefined, files.submitted.length > 0 ? `- Handed in:\n${files.submitted.map((file) => ` - ${formatFileLine(file)}`).join('\n')}` : '- No files attached to the submission', @@ -241,11 +332,12 @@ export async function describeSubmission( ].filter(Boolean) as string[]; return joinSections([ - heading(3, 'Your submission'), + // A teacher sees other people's work here, so do not call it "yours". + heading(3, mine.length > 0 ? 'Your submission' : 'Submissions'), parts.join('\n\n'), ...written, 'Read any attachment with download_file.' + - (written.length === 0 && anyGraded(relevant) + (written.length === 0 && !page?.grading.some((entry) => entry.gradeComment) && anyGraded(relevant) ? ' No written feedback was found for this submission. It is read from the web page rather ' + 'than an API, so treat this as "not found", not as "none was given" — the teacher may ' + 'have responded on paper or in person.' diff --git a/src/store/store.ts b/src/store/store.ts index 5b68aef..74600a5 100644 --- a/src/store/store.ts +++ b/src/store/store.ts @@ -14,7 +14,7 @@ import { connect, migrate, type Db } from './db.ts'; * Identity diffing also gives deletions for free, which no timestamp scheme can. */ -export type NodeKind = 'course' | 'room' | 'board' | 'lesson' | 'task' | 'file'; +export type NodeKind = 'course' | 'room' | 'board' | 'lesson' | 'task' | 'file' | 'submission'; export interface StoredNode { kind: NodeKind; @@ -36,6 +36,8 @@ export interface SearchResult { path: string; snippet: string; rank: number; + /** The node's stored metadata — a submission carries its `taskId` here. */ + meta?: Record; } export interface DiffResult { @@ -227,22 +229,29 @@ export class Store { const kinds = options.kinds ?? null; const fts = await this.db.query( - `SELECT kind, node_id, course_id, course_title, title, path, - ts_rank(fts, q) AS rank, - ts_headline('german', coalesce(nullif(body, ''), title), q, + // `meta` lives on nodes, not on search_docs; joined rather than + // duplicated so the two cannot drift apart. + `SELECT d.kind, d.node_id, d.course_id, d.course_title, d.title, d.path, n.meta, + ts_rank(d.fts, q) AS rank, + ts_headline('german', coalesce(nullif(d.body, ''), d.title), q, 'MaxWords=32, MinWords=8, MaxFragments=1, StartSel=**, StopSel=**') AS snippet - FROM search_docs, websearch_to_tsquery('german', $2) q - WHERE crawl_id = $1 AND fts @@ q AND ($3::text[] IS NULL OR kind = ANY($3)) + FROM search_docs d + LEFT JOIN nodes n + ON n.crawl_id = d.crawl_id AND n.kind = d.kind AND n.node_id = d.node_id, + websearch_to_tsquery('german', $2) q + WHERE d.crawl_id = $1 AND d.fts @@ q AND ($3::text[] IS NULL OR d.kind = ANY($3)) ORDER BY rank DESC LIMIT $4`, [crawlId, query, kinds, limit], ); const trgm = await this.db.query( - `SELECT kind, node_id, course_id, course_title, title, path, - similarity(title, $2) AS rank, - title AS snippet - FROM search_docs - WHERE crawl_id = $1 AND title %> $2 AND ($3::text[] IS NULL OR kind = ANY($3)) + `SELECT d.kind, d.node_id, d.course_id, d.course_title, d.title, d.path, n.meta, + similarity(d.title, $2) AS rank, + d.title AS snippet + FROM search_docs d + LEFT JOIN nodes n + ON n.crawl_id = d.crawl_id AND n.kind = d.kind AND n.node_id = d.node_id + WHERE d.crawl_id = $1 AND d.title %> $2 AND ($3::text[] IS NULL OR d.kind = ANY($3)) ORDER BY rank DESC LIMIT $4`, [crawlId, query, kinds, limit], ); @@ -262,6 +271,7 @@ export class Store { path: row.path, snippet: (row.snippet ?? '').replace(/\s+/g, ' ').trim(), rank: Number(row.rank), + meta: (row.meta ?? undefined) as Record | undefined, }); } return merged.sort((a, b) => b.rank - a.rank).slice(0, limit); @@ -446,6 +456,7 @@ interface SearchRow { path: string; snippet: string | null; rank: string; + meta: Record | null; } function toNode(row: NodeRow): StoredNode { @@ -521,6 +532,35 @@ export function snapshotToNodes(snapshot: Snapshot): StoredNode[] { } } + // Submissions, when the crawl was asked for them. The digest deliberately + // includes the grade and the feedback: re-grading a submission changes + // neither its id nor its text, so without them what_changed would never + // report the one event a student actually waits for. + for (const submission of snapshot.submissions ?? []) { + nodes.push({ + kind: 'submission', + nodeId: submission.id, + courseId: submission.courseId, + title: submission.taskName, + body: [submission.submittedText, submission.gradeComment].filter(Boolean).join('\n\n'), + path: `${submission.courseTitle}/${submission.taskName}`, + meta: { + taskId: submission.taskId, + isSubmitted: submission.isSubmitted, + isGraded: submission.isGraded, + grade: submission.grade ?? null, + hasFeedback: Boolean(submission.gradeComment), + }, + digest: digestOf([ + submission.isSubmitted, + submission.isGraded, + submission.grade ?? null, + submission.gradeComment ?? '', + submission.submittedText ?? '', + ]), + }); + } + // Rooms sit alongside courses rather than inside them. `course_id` carries // the room id: the column is the container key, and widening its meaning // keeps per-container carry-forward and the manifest working unchanged. diff --git a/test/homework-page.test.ts b/test/homework-page.test.ts index e14570b..1b12cf7 100644 --- a/test/homework-page.test.ts +++ b/test/homework-page.test.ts @@ -1,6 +1,6 @@ import assert from 'node:assert/strict'; import { describe, it } from 'node:test'; -import { parseHomeworkPage } from '../src/core/homework-page.ts'; +import { parseHomeworkPage, parseTeacherGrading } from '../src/core/homework-page.ts'; /** * Fixtures mirror the legacy client's templates (feedback.hbs, submission.hbs) @@ -92,3 +92,63 @@ describe('parseHomeworkPage', () => { assert.equal(parseHomeworkPage(html)?.submittedFiles[0]?.name, 'A&B "final".pdf'); }); }); + +/** + * The teacher's grading form, as the legacy client renders it. + * + * Anchored on the `name=` attributes the POST handler reads rather than on + * layout: one hidden `submissionId` per block, `teamMembers` naming who handed + * it in, a `grade` number input whose `value` is empty when ungraded (the + * `placeholder` is a hint, not a grade), and a `gradeComment` textarea whose + * body arrives HTML-escaped. + */ +const gradingBlock = ( + submissionId: string, + submitterId: string, + grade: string, + comment: string, +) => ` + + +

+ + + + + +
`; + +describe('parseTeacherGrading', () => { + const a = 'a'.repeat(24); + const b = 'b'.repeat(24); + const student1 = '1'.repeat(24); + const student2 = '2'.repeat(24); + + it('reads every submission on the form, with its submitter', () => { + const html = `
${gradingBlock(a, student1, '', '<p>Alles richtig!</p>')}${gradingBlock(b, student2, '100', '<p>Alles korrekt!</p>')}
`; + const grading = parseTeacherGrading(html); + assert.equal(grading.length, 2); + assert.deepEqual(grading[0]?.submitterIds, [student1]); + assert.deepEqual(grading[1]?.submitterIds, [student2]); + }); + + it('distinguishes a feedback-only grade from a percentage', () => { + const html = gradingBlock(a, student1, '', '<p>Alles richtig!</p>'); + const [entry] = parseTeacherGrading(html); + // An empty value is ungraded; the placeholder "95" must not be read as one. + assert.equal(entry?.gradePercent, undefined); + assert.equal(entry?.gradeComment, 'Alles richtig!'); + }); + + it('reads a percentage when one was given', () => { + const [entry] = parseTeacherGrading(gradingBlock(b, student2, '100', '<p>Gut</p>')); + assert.equal(entry?.gradePercent, 100); + assert.equal(entry?.gradeComment, 'Gut'); + }); + + it('returns nothing for the student view, which has no grading form', () => { + assert.deepEqual(parseTeacherGrading(page({ feedback: '
Gut
' })), []); + }); +}); diff --git a/test/render.test.ts b/test/render.test.ts index 5a85d09..3b57ac4 100644 --- a/test/render.test.ts +++ b/test/render.test.ts @@ -23,7 +23,35 @@ describe('htmlToText', () => { }); it('decodes entities, ampersand last so &lt; stays literal', () => { - assert.equal(htmlToText('

a &lt; b < c  d

'), 'a < b < c d'); + assert.equal(htmlToText('

a &lt; b < c

'), 'a < b < c'); + }); + + it('collapses runs of spaces, including the non-breaking ones', () => { + // Schulcloud content is full of   used as padding; keeping it would + // reproduce that padding in the plain-text output for no benefit. + assert.equal(htmlToText('

a  b

'), 'a b'); + }); + + it('strips the source template\'s indentation from every line', () => { + // Paragraphs still separate with a blank line; what goes is the leading + // run of spaces the server-side template left on each line. + const html = '

\n first line
\n second line

'; + assert.equal(htmlToText(html), 'first line\nsecond line'); + }); + + it('separates table cells so columns do not run together', () => { + const html = '' + + '
BestandteilFunktion
Gehirnsteuert
'; + assert.equal(htmlToText(html), 'Bestandteil | Funktion\nGehirn | steuert'); + }); + + it('keeps a cell that wraps its text in a paragraph on one row', () => { + const html = '

Bestandteil

Gehirn
'; + assert.equal(htmlToText(html), 'Bestandteil | Gehirn'); + }); + + it('separates paragraphs with a blank line', () => { + assert.equal(htmlToText('

first

\n

second

'), 'first\n\nsecond'); }); it('returns an empty string for missing input', () => {