Conversation
There was a problem hiding this comment.
Pull request overview
Updates the backend API source-of-truth OpenAPI spec to reflect Tutorial v2’s transitional “expand” contract for Course/CourseSeries kinds and content, and documents the new playground Copilot-context generation endpoint.
Changes:
- Extends
CourseandCourseSeriesschemas withkind, and addsCourse.contentas the kind-specific opaque payload. - Updates course and course-series create/update/list endpoints to document transitional legacy behavior and adds
kindfiltering for series list endpoints. - Adds
POST /user/courses/playground/copilot-contextfor generating an editable Copilot context draft from unsaved playground content.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
FennoAI Review — PR #3471 (OpenAPI: course kind / playground content)
The change is a well-structured, single-file extension of docs/openapi.yaml. It cleanly introduces the guided/playground kind enum for Course and CourseSeries, a new content field, a new POST /user/courses/playground/copilot-context endpoint, and kind query filters, while documenting the transitional legacy (entrypoint/prompt) shape.
Strengths
kindenums are single-sourced via$refand consistent betweenCourseandCourseSeries.- The
kindquery filter is applied uniformly across all three course-series list endpoints with identical wording. - The new endpoint documents the
courseAdminrestriction, the quota/refund behavior, and a complete 400/401/403/429 response set, inheriting the globalbearerAuthscheme correctly.
Cross-cutting notes (no reliable single line):
- Unbound "API contract release" reference. The phrasing "accepted/removed until/by the API contract release" appears in several places (createCourse, updateCourse, createCourseSeries, and the deprecated fields) but is never resolved to a concrete version/date. Consider defining the target release once so readers can determine when the transitional shape disappears.
- SSRF / arbitrary-fetch surface (backend verification). The playground
contentvalues useformat: uri-reference, which permits relative and arbitrary-scheme URIs. If the backend dereferences these caller-supplied "universal URLs" while assembling copilot context, ensure it allowlists accepted schemes (e.g.kodo://,data:) and rejectshttp(s)/file/etc. This cannot be enforced in the spec — please confirm against the handler. - Quota refund-on-failure. "Refunded when generation fails" is a common spot for double-spend / refund-race bugs and, combined with unbounded input (see inline), can allow repeated expensive-but-failing submissions without depleting quota. Verify the refund is atomic and that per-request cost is bounded before the expensive work runs.
There was a problem hiding this comment.
FennoAI Review — PR #3471 (continued)
Two additional inline findings that did not fit in the first review submission. See the prior review for the summary and cross-cutting notes.
Additional findings
docs/openapi.yaml:2867: [P3] kind+content vs legacy shape exclusivity is prose-only: The description says the request "either provideskindandcontent, or uses the transitional legacy flat shape (entrypointandpromptwithoutkindandcontent)", but the schema declares all four as flat optional properties with onlytitle/thumbnailrequired. A codegen consumer cannot tell that mixingcontentwithentrypoint/prompt, orkind: playgroundwithentrypoint, is invalid, nor that one of the two shapes is effectively required.
|
对 review 正文里三条横切备注的处理:
|
…oneOf A guided content document is itself a string-to-string map, so it also satisfies the playground file-collection shape and a oneOf on content would reject valid guided courses (the point made in the goplus#3471 review). Follow the AIGC settings precedent instead: Course and the create/update bodies keep a flat object with an allOf of if/then branches keyed on kind, which is what selects GuidedCourseContent or PlaygroundCourseContent.
| $ref: "#/components/schemas/Course/properties/title" | ||
| thumbnail: | ||
| $ref: "#/components/schemas/Course/properties/thumbnail" | ||
| content: |
There was a problem hiding this comment.
这里应该可以复用 PlaygroundCourseContent:
content:
description: Current, possibly unsaved, playground course content.
$ref: "#/components/schemas/PlaygroundCourseContent"
minProperties: 1| owner: | ||
| description: Username of the course series' owner. | ||
| $ref: "#/components/schemas/User/properties/username" | ||
| kind: |
There was a problem hiding this comment.
这个 kind 在 Course 和 CourseSeries 里重复定义了,可以考虑提取一个 CourseKind,然后它俩 $ref
| description: | | ||
| Kind-specific course content, selected by `kind`: `GuidedCourseContent` for a `guided` course, | ||
| `PlaygroundCourseContent` for a `playground` course. The API validates only the top-level shape; internals | ||
| follow the Tutorial content-format contract and stay opaque to the API and storage. |
There was a problem hiding this comment.
这个 Tutorial content-format contract 有固定的文档链接吗?或者 issue 之类的
There was a problem hiding this comment.
稍等,我把链接发出来,是在代码库里面(
| PlaygroundCourseContent: | ||
| description: | | ||
| Content of a `playground` course, driven by course code: a file collection mapping file paths to universal URLs. | ||
| The referenced files form the Tutorial project; its layout is owned by the Tutorial Class Framework contract. An |
There was a problem hiding this comment.
Tutorial Class Framework contract 还有这个也可以补上链接,如果有的话
Sync the API spec with the Tutorial v2 backend changes (goplus/builder-backend#350/goplus#353/goplus#354), documenting the transitional expand-release shape: - Course gains kind (immutable, guided | playground) and kind-specific opaque content; the top-level entrypoint/prompt copies are marked deprecated and noted for removal by the API contract release. - Course create/update accept both the kind+content shape and the transitional legacy flat payload; update documents kind immutability. - CourseSeries gains kind; series lists gain a kind filter; series create/update document the kind semantics. - New POST /user/courses/playground/copilot-context endpoint. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Aligns with GeneratePlaygroundCourseCopilotContextInput in module_CourseApis.ts and the backend validation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The endpoint draws from the copilotMessage quota, so its 403/429 responses follow the same contracts as the other quota-backed endpoints (40301/42901 with Retry-After). Also state the refund policy precisely: server-side failures refund, client cancellation does not. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Course.content now shows both kind-specific shapes, and every mention of the transitional shape's removal points at the concrete contract PR (goplus/builder-backend#352) instead of an unnamed future release. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…guided too Matches goplus/builder-backend#353: the flat entrypoint/prompt shape applies to guided creates whether kind is omitted or given, and content wins when both are present.
Pairs with goplus/builder-backend#352 (migration 018 plus its application change): the transitional shapes introduced for the expand phase are gone. - `Course` drops the deprecated top-level `entrypoint` / `prompt`; `content` becomes a `oneOf` of the new `GuidedCourseContent` and `PlaygroundCourseContent` schemas, selected by `kind`. - `POST /user/courses` requires `kind` and `content`; the legacy flat payload is no longer documented. `PATCH /courses/{id}` no longer accepts the flat fields and states that `content` is replaced as a whole. - `POST /user/course-series` requires `kind`. BREAKING CHANGE: clients must send `kind` + `content` (and `kind` on course series) and must read guided course data from `content`.
…oneOf A guided content document is itself a string-to-string map, so it also satisfies the playground file-collection shape and a oneOf on content would reject valid guided courses (the point made in the goplus#3471 review). Follow the AIGC settings precedent instead: Course and the create/update bodies keep a flat object with an allOf of if/then branches keyed on kind, which is what selects GuidedCourseContent or PlaygroundCourseContent.
Review feedback from the course API spec sync: - The copilot-context request body repeated the playground file-collection shape; it now references PlaygroundCourseContent, with minProperties: 1 expressing the one difference (a course may be saved empty, a context request may not be). - The guided/playground enum was written twice; it becomes a shared CourseKind schema, following the Visibility precedent, with the kind-specific wording staying at each use site. - The "Tutorial content-format contract" and "Tutorial Class Framework contract" mentions now link to the design docs they name. Requested by review (aofei). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
GET /courses, GET /user/courses and GET /users/{username}/courses accept an
optional `kind` query parameter, mirroring the course series lists. Without
it a client has to filter after paginating, which leaves pages short or
empty once both kinds exist while `total` still counts every kind.
Backend: goplus/builder-backend#353.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
ListCoursesParams gains an optional `kind`, mirroring ListCourseSeriesParams. Both kinds now share the course lists, so a consumer that wants one kind needs the server to filter before paginating; filtering a page client-side leaves it short while `total` still counts every kind. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Types the `kind` query parameter of the course list endpoints (goplus/builder-backend#353) on ListCoursesParams. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
b6286a9 to
ac83c29
Compare
UpdateCourseSeriesParams in the Tutorial v2 contract carries no kind (9822f64), so a series keeps the kind it was created with. The spec had still described changing it. PATCH now documents kind the way the course PATCH does: an echoed, unchanged kind is accepted, a different one is rejected. Both create and update state the membership rule. Backend: goplus/builder-backend#353. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The course management modal and the course series edit modal paginated the signed-in user's courses and then dropped non-guided ones client-side. Once playground courses exist that breaks both: - The management modal pages by 8 ordered by updatedAt desc. With the 8 most recently updated courses all being playground courses, its first page showed "no results" while the pager still counted every kind. - The series edit modal takes the first 100 courses; enough playground courses push guided ones out of that window. Both now pass `kind: 'guided'` so the server filters before paginating. The `filter(isGuidedCourse)` stays only to narrow `Course[]` to `GuidedCourse[]`. Requires the backend filter from goplus/builder-backend#353. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Syncs
docs/openapi.yaml(the backend API spec source of truth) with the Tutorial v2 course storage and API changes. Part of #3422. Backend: goplus/builder-backend#350 / goplus/builder-backend#353 / goplus/builder-backend#354 (expand release) and goplus/builder-backend#352 (contract release).The spec documents the final contract as specified by
docs/develop/tutorial-v2/module_CourseApis.ts, not the transitional shapes of the rollout: the document has no machine consumers (the frontend client is hand-written, nothing generates from it), so there is no reason to publish an intermediate spec. Until goplus/builder-backend#352 is deployed, the Release 1 backend serves a superset of this spec — it still returns top-levelentrypoint/prompton guided courses, still accepts the flat{entrypoint, prompt}create/update payload, and defaults an omitted serieskindtoguided. A client written against this spec works on both releases; nothing new needs the superset.Coursegainskind(immutable,guided | playground) and kind-specificcontent.contentis selected bykindthroughallOf/if/thenbranches (theAIGCEnrich…settings precedent) intoGuidedCourseContent(entrypoint,prompt) orPlaygroundCourseContent(a file collection mapping paths to universal URLs; may be empty so a course can be created before its files are uploaded). NotoneOf: a guided content document is itself a string→string map and would satisfy both alternatives.kind+content; update replacescontentas a whole after validating it against the course's kind and documents kind immutability.CourseSeriesgainskind(required on create); the three series list endpoints gain akindquery filter; the series kind is immutable on update, and both create and update state the member-kind consistency rule.POST /user/courses/playground/copilot-contextendpoint (courseAdmin gate,copilotMessagequota, refunded on failure).API surface changes (as documented in this spec)
No new routes besides the copilot-context endpoint — a Playground Course is a
kindof the existing course resource, so support lands as payload changes on the existing endpoints.POST /user/courses{kind, title, thumbnail, content};kind: guidedwithGuidedCourseContent,kind: playgroundwithPlaygroundCourseContent. Response gainskind/content.PATCH /courses/{courseID}content(validated against the course's kind, replaced as a whole);kindmay be echoed but is immutable.GET /courses/{courseID},GET /courses,GET /user/courses,GET /users/{username}/coursesCourseschema gainskindandcontent; the pre-kind top-levelentrypoint/promptare gone from the spec (the Release 1 backend still returns them, see above).GET /courses,GET /user/courses,GET /users/{username}/courseskindquery parameter (goplus/builder-backend#353).DELETE /courses/{courseID}POST /user/course-series,PATCH /course-series/{courseSeriesID}kindrequired on create and immutable afterwards (PATCHaccepts only an echoed, unchangedkind); both state that every course incourseIDsmust have the series' kind.GET /course-series,GET /user/course-series,GET /users/{username}/course-serieskindquery parameter;CourseSeriesschema gainskind.GET /course-series/{courseSeriesID},DELETE /course-series/{courseSeriesID}kind; otherwise unchanged.POST /user/courses/playground/copilot-context{title, thumbnail, content}→{copilotContext};courseAdmingate;copilotMessagequota withForbiddenOrQuotaExceeded/RateLimitExceededresponses (40301/42901 +Retry-After); server-side failures refund, client cancellation does not.Beyond the spec, this PR carries three small companions to the course list
kindfilter, each in its own commit so any can be dropped:docs/develop/tutorial-v2/module_CourseApis.tsgainskind?: CourseKindonListCoursesParams(an amendment to the design contract, mirroringListCourseSeriesParams),spx-gui/src/apis/course.tstypes the same parameter, and the two call sites that filtered guided courses client-side after paginating (CourseManagementModal.vue,CourseSeriesEditModal.vue) now passkind: 'guided'. The last one fixes a visible bug: with the most recently updated courses all being playground courses, the management modal's first page showed "no results" while the pager counted every kind. It needs goplus/builder-backend#353 deployed to take effect; against an older backend the parameter is ignored and behavior is unchanged.The documented 400s and response shapes were verified against the Release 2 backend binary in the goplus/builder-backend#352 rehearsal (see its description).