chore(ts-sdk): regenerate Knowledge Graph API types from upstream spec - #158
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
Regenerates the TypeScript Knowledge Graph OpenAPI type definitions to match the current upstream spec, and adds a conformance test to prevent future spec/type drift from silently changing SDK behavior.
Changes:
- Regenerated
kg-api.d.tsfrom the upstream Knowledge Graph OpenAPI spec (notably including the new requiredhasChildrenfield onStandardsFrameworkItemand enum-type tightening). - Added a unit test that type-checks and runtime-checks the seam between generated spec types and hand-written SDK types/projections.
- Excluded the generated
kg-api.d.tsfrom ESLint to avoid upstream-doc-text characters breaking lint (e.g., NBSP / irregular whitespace).
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| sdks/typescript/src/knowledge-graph/kg-api.d.ts | Updated generated OpenAPI types to match the live upstream Knowledge Graph spec. |
| sdks/typescript/tests/unit/knowledge-graph/spec-conformance.test.ts | Adds compile-time and runtime guards to detect spec/type drift and projection changes. |
| sdks/typescript/eslint.config.js | Ignores the generated OpenAPI types file to prevent upstream doc text from failing lint. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
czi-fsisenda
left a comment
There was a problem hiding this comment.
LGTM! 🚀
We should add kg-api.d.ts to a .gitattributes file as a generated file too
|
Good call — done in Also added the file to the header comment listing each generated path and the command that produces it ( Verified with |
Regenerates
sdks/typescript/src/knowledge-graph/kg-api.d.tsfrom the live spec atdocs.learningcommons.org, which had drifted from the checked-in copy. Split out from #157 to keep that security bump scoped.What actually changed in the spec
The raw diff is ~1800 lines, but almost all of it is the curriculum path block (
/courses,/lessons,/assessments, …) moving from the top ofpathsto the bottom. All 26 endpoints are unchanged. Filtering comments and ignoring order leaves three real changes, all inStandardsFrameworkItem:hasChildren: booleanadded (required)gradeLevel?: string[]→GradeLevelENUM[] | null(6 sites)types.tswidens tostring[]by designjurisdiction?: string | null→JurisdictionENUMGradeLevelENUMandJurisdictionENUMmember lists are unchanged. The narrowing is absorbed becausetypes.tsdeliberately declares these asstring/string[]so the API can return values outside the current spec — the comment at the top of that file already states this.Verification
tsc --noEmit, lint, and build clean; 321 tests pass (318 + 3 new). Also test-merged into the open KG PRs — clean merge and green on both:sdk-kg-hardening) — 346 passsdk-standards-api, stacked on fix: harden Knowledge Graph client pagination, code normalization, and ambiguity detection #148) — 380 passBased on
mainrather than stacked: no open PR toucheskg-api.d.tsoreslint.config.js, so there is nothing to conflict with.New test:
tests/unit/knowledge-graph/spec-conformance.test.tsGuards the seam between the generated types and the hand-written ones in
types.ts, so the next regeneration fails loudly instead of silently changing behaviour. Both type-level guards were verified to actually fail:Jurisdictionenum ↔ specJurisdictionENUMmutual assignability. All 52 values currently match exactly. Confirmed a bogus added member failstsc. This is the one real gap —client.tsonly ever casts jurisdiction, so nothing else would have caught drift in that hand-maintained enum.getStandardsByGradeprojection mirrored against the spec schema; confirmed a renamed spec field failstsc.hasChildren) still projects to exactly the sixAcademicStandardkeys, so future required-field additions can't leak into the public shape.Lint config
no-irregular-whitespacebegan erroring on the regenerated file — the upstream spec text contains a U+00A0 in astatementCodedescription. Rather than strip the character (it would return on the next regeneration), the generated file is now excluded from lint. Nothing hand-maintained lives in it, and upstream prose shouldn't be able to break our lint gate.Notes for follow-up (not in this PR)
scripts/checks/or CI regenerates and diffskg-api.d.ts, which is why it went stale. A check would need to hit the live spec URL, coupling CI to an external service — worth deciding deliberately rather than bundling here.PaginatedResponse.datais typedRecord<string, never>[]in the spec, intersected with the concreteStandardsFrameworkItem[]override. Reading through the intersection works, but the element type can't be constructed, which is why the fixtures pin to the schema directly. Upstream spec wart worth reporting.hasChildrenmay simplify fix: harden Knowledge Graph client pagination, code normalization, and ambiguity detection #148/feat: add public StandardsCatalog for listing and validating academic standards #149. They distinguish organizational groupings from leaf standards vianormalizedStatementTypeplus learning-component-set comparison;hasChildrennow exposes this directly. Adjacent, not overlapping — no change made here.