feat: add public StandardsCatalog for listing and validating academic standards - #149
feat: add public StandardsCatalog for listing and validating academic standards#149adnanrhussain wants to merge 1 commit into
Conversation
982161d to
7a60394
Compare
9963192 to
83295a4
Compare
009d334 to
0192bb5
Compare
There was a problem hiding this comment.
Pull request overview
Introduces a new public TypeScript SDK surface for listing and validating academic standards via a StandardsCatalog, while hardening the underlying Knowledge Graph client’s normalization, ambiguity detection, and pagination behavior.
Changes:
- Adds a public
StandardsCatalogwithlistStandards,getStandard, andvalidateCodes, plus related exported types. - Enhances
KnowledgeGraphClientwith statement-code normalization, ambiguity flagging (limit=2), and shared cursor-pagination handling. - Updates math standards alignment evaluator/tests to reflect the KG client option changes (removal of caller-provided
limit).
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| sdks/typescript/src/knowledge-graph/standards-catalog.ts | New public catalog wrapper over KG lookups, including bulk code validation behavior. |
| sdks/typescript/src/knowledge-graph/client.ts | Adds normalization helper, ambiguity detection, cursor pagination utility, and updated return shapes. |
| sdks/typescript/src/knowledge-graph/types.ts | Extends StandardInfo with required statementCode and normalizedCode, plus ambiguous. |
| sdks/typescript/src/knowledge-graph/index.ts | Re-exports catalog and normalization symbols/types from the KG module barrel. |
| sdks/typescript/src/index.ts | Exposes StandardsCatalog, normalizeStatementCode, and related types/errors from the package entrypoint. |
| sdks/typescript/src/errors.ts | Adds StandardNotFoundError and allows KnowledgeGraphError to carry distinct error codes. |
| sdks/typescript/src/evaluators/math/standards-alignment.ts | Removes deprecated KG limit option usage and adapts to updated KG client behavior. |
| sdks/typescript/tests/unit/knowledge-graph/standards-catalog.test.ts | New unit coverage for catalog listing and bulk validation semantics. |
| sdks/typescript/tests/unit/knowledge-graph/client.test.ts | Updates/extends KG client unit tests for ambiguity, normalization, and pagination behavior. |
| sdks/typescript/tests/unit/evaluators/math/standards-alignment.test.ts | Updates expected KG call options after limit removal. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
83295a4 to
d7f8c92
Compare
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
d7f8c92 to
3e8302f
Compare
0192bb5 to
bb2154e
Compare
3e8302f to
b75d61c
Compare
bb2154e to
3a3777f
Compare
b75d61c to
84f61c2
Compare
3a3777f to
8ffea5b
Compare
d5dbd18 to
4db7537
Compare
|
Five of six fixed in Fixed
Declined: 400 as fatal. After the local guards, the plausible global cause is an invalid Tests extended rather than added where possible; zero surviving mutants in every changed region, catalog now at 88.7% on covered code. |
4db7537 to
b1b5550
Compare
|
Both addressed in
Finding 2 — the misleading-answer half is fixed: a 400 now returns Still not fixed, deliberately: it issues N requests before saying so, and The clean fix is a third option neither of us proposed: reuse |
b1b5550 to
cfebded
Compare
Stacked on #148
Why
Nothing public can list, validate, or resolve standards (
src/index.tsexports onlyJurisdiction). Batch can't check astatement_codescolumn before spending LLM calls on it, and consumers reimplement the REST calls —demos/typescript/server/kg.tssays so in a comment, and carried the same pagination bug #148 fixes.What
StandardsCatalog—listStandards,getStandard,resolveStandard,validateCodes.KnowledgeGraphClientstays internal so its cache and concurrency semantics don't become API.The resolver, and why the classification has five states
A statement code does not reliably identify one standard. From a read-only scan of the production KG, Mathematics has 1,950 of 43,019 (jurisdiction, code) pairs matching multiple standards — worst case 40. Utah
F.IF.7.bmatches five, differing only by course (Secondary Math I/II/III, Honors, Precalculus) with 3/5/0/0/10 learning components. All five share identicalisCurrent,adoptionStatus,normalizedStatementTypeandgradeLevel, and the course lives on an ancestor node that search cannot filter on. So no query fixes this; it has to be resolved client-side.resolveStandardfetches all candidates, discards those with no learning components, and classifies what remains:resolvedno-learning-componentsambiguouscandidates[]is returned so the caller picks auuidnot-founduncheckedThat takes genuine guessing from 1,950 cases to 82 — and makes
0/0mean one specific thing instead of two.Interchangeability is decided on learning-component sets, not descriptions. 13 Mathematics codes have candidates whose descriptions read identically but whose components differ — Georgia
A.PAR.4.1has 14 versus 2. Comparing text would have called those safe.Notes for review
validateCodesreturns five states, not a boolean. A boolean forces a network blip into "the code is bad". OnlyAuthenticationError/ConfigurationErrorthrow, since those doom every other lookup identically; the rest yielduncheckedand the remaining codes still resolve.Promise.allhere would have reintroduced the all-or-nothing failure the sibling PR removes fromevaluateItems.StandardNotFoundErrorcarriesSTANDARD_NOT_FOUND, not the inheritedKNOWLEDGE_GRAPH_ERROR, so consumers recordingerror.codecan separate a typo from an outage.truncatedis set when the candidate list hit the search limit — 5 (jurisdiction, code) pairs across all subjects exceed 50, max 87, and search takes no cursor.academicSubjecthas no default — an unfiltered grade listing mixes math and ELA, and defaulting to'Mathematics'would bake the current single family into a general API.Empty and over-long (>50 char) codes are rejected locally. Observed max real length is 45, so that bound rejects nothing valid.
validateCodesaborts early on a fatal error. A bad key fails every code identically, so queued lookups short-circuit rather than issuing N doomed requests. The check sits behind apLimitdeliberately — in front of it, every lookup is already in flight before the first rejection arrives.Constructor validates its inputs: non-integer or sub-1
concurrencythrowsConfigurationErrorrather than crashing inside p-limit, and a whitespace-onlyacademicSubjectis treated as absent instead of being sent as a filter that matches nothing.Cost: for the 95.5% of codes with one candidate this is one extra learning-component fetch that the evaluator would make anyway. Ambiguous codes cost one fetch per candidate, cached, so repeats across CSV rows are free.
A standard is never called unauthored when its components merely lack descriptions. The KG
documents
descriptionas optional and alignment is judged from that text, so such components areunevaluable — but reporting "no learning components are authored" for them is simply false. The status
stays
no-learning-components(nothing evaluable either way) while the message reports what wasactually observed, and
undescribedComponentCountis set so a report can separate a data-quality gapfrom a genuinely empty standard. Depends on
getLearningComponentSetfrom fix: harden Knowledge Graph client pagination, code normalization, and ambiguity detection #148.Not included
_evaluateCoreto the resolver is a follow-up so that behaviour change stays independently revertable. fix: harden Knowledge Graph client pagination, code normalization, and ambiguity detection #148 warns when a code is ambiguous in the meantime.^0.8.0, so that's a post-release follow-up.Verification
npm run lint(0 errors),typecheck,test:unit— 377 passing, 34 in the catalog suite.buildconfirmsresolveStandard,CodeResolutionStatusandStandardCandidatereachdist/index.d.ts.Mutation tested with Stryker: 88% on covered code, with the resolver's classification branches, the early-abort check, and both constructor validations all confirmed killed.