Skip to content

fix(sdk): surface a swallowed KG failure, repoint the report grade filter, honour VC's declared conditions - #247

Merged
adnanrhussain merged 3 commits into
mainfrom
sdk-fix-four
Aug 30, 2026
Merged

fix(sdk): surface a swallowed KG failure, repoint the report grade filter, honour VC's declared conditions#247
adnanrhussain merged 3 commits into
mainfrom
sdk-fix-four

Conversation

@adnanrhussain

Copy link
Copy Markdown
Collaborator

Four independent findings from the code review. Items 2 and 7 follow in their own PRs; 6 and 8 are deferred.

1. A swallowed Knowledge Graph failure reported as real data

math-standards-alignment.ts prefetched learning components with .catch(() => undefined). A miss then fell through to totalCount: cached?.components.length ?? 0, so a failed KG call reported 0/0 — indistinguishable from a standard that genuinely has no learning components.

The failure now travels on the result's error, which already means exactly this: "alignedCount is 0 because nothing was measured, not because nothing aligned." A warning is logged too. totalCount still reads 0, but a caller can now tell why.

3. The standards report's grade filter emptied the table

standards-report.html filtered and displayed it.grade; the formatter emits gradeLevel. Since it.grade was always undefined, selecting a grade filtered out everything rather than filtering. Three uses repointed.

Nothing caught it because the filter is browser JS in a template that no test executes. It is now asserted on the template text — no bare it.grade, it.gradeLevel present, and the formatter still emits that field — the same approach the evaluator-ordering check uses.

4. JSDoc that miscounted by 3×

readability.ts said "see PREPROCESSING_GAPS … for the three evaluators that call this instead." #237 migrated two of them: there is one caller and one gap entry. A stale pointer is worse than none, because it is the one people follow.

5. vocabulary-complexity ignored its own declared conditions — in three places

The contract declares fk_score conditional on grades 3-4, and the two complexity steps mutually exclusive on the same grades. The SDK restated that rule rather than reading it:

location was
evaluator computed FK for every grade, including the 5-12 branch whose prompt does not bind it
prompts/…/user.ts studentGradeLevel === '3' || === '4' chose the template
prompts/…/system.ts the same literal chose the system prompt

All three now read the contract. grep for === '3' in the evaluator or prompt layers returns nothing.

Also fixed: getUserPrompt called fkLevel.toString() unconditionally, so an absent score would have put the string "undefined" into any template carrying the token.

Two tests I had to rewrite because they proved nothing

Worth recording, since both passed on the first attempt and neither could fail.

The condition tests. I first asserted the rendered prompt contained no {fk_score} and no "undefined". Both hold whether or not the condition is honoured — the other-grades template never carried the token. Reverting all three fixes left 14/14 passing. They now assert what is observable: that the branch selection differs by grade, and that the grades 5-12 prompt renders identically with or without a score. Each fix reverted fails one.

The prefetch test. It asserted the coarse-filtered result carried the error — and passed with the fix reverted, because the coarse mock marked the standard relevant, so it reached the evaluation error path instead. The mock's verdict is now controllable and the test forces relevant: false, plus asserts coarseFiltered === true so it cannot silently drift back to the other branch.

Validation

  • typecheck, lint, test:unit (1129, up from 1121), build, scripts/check.py
  • Live: 10/10 across vocabulary-complexity and math, the two evaluators changed
  • Every fix confirmed load-bearing by reverting it: system-prompt branch → 1 fails; user template branch → 1; unguarded fk_score → 1; report grade field → 1; prefetch error → 1
  • One unrelated flake seen once in a full run: generate-schema.test.ts spawns a subprocess and timed out at 6959ms under load, passing 38/38 in isolation. Not touched here.

One test assertion loosened

processingTimeMs > 0 in the vocabulary-complexity test began failing: with both providers mocked and the 5-12 branch no longer computing a readability score, the evaluation finishes inside one clock tick. It now asserts >= 0, with the reason recorded — a positive wall-clock on a fully mocked path was measuring that FK happened to take a millisecond, not anything about the product.

Copilot AI lite review requested due to automatic review settings August 30, 2026 03:59
@codecov

codecov Bot commented Aug 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR tightens SDK conformance to evaluator contracts and fixes a few correctness gaps: it surfaces Knowledge Graph (KG) prefetch failures instead of silently reporting 0/0, corrects the standards report grade filter to use the formatter’s emitted field, and ensures vocabulary-complexity routing / preprocessing behavior is derived from the contract rather than hardcoded.

Changes:

  • Preserve and report KG learning-component prefetch failures on coarse-filtered math standards alignment results (with logging + unit test coverage).
  • Fix the standards report template to filter/render gradeLevel (and assert template/formatter agreement via unit test).
  • Make vocabulary-complexity branch selection and FK preprocessing conditional follow config.json (prompt layer + evaluator), plus update related tests and stale readability JSDoc.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
sdks/typescript/src/evaluators/academic-standards-alignment/mathematics/math-standards-alignment.ts Record KG prefetch failures and surface them on coarse-filtered results.
sdks/typescript/tests/unit/evaluators/math-standards-alignment.test.ts Add regression test ensuring KG prefetch failure is observable on coarse-filtered path.
sdks/typescript/src/batch/families/standards-report.html Use gradeLevel for grade filtering and display in the standards report template.
sdks/typescript/tests/unit/batch/standards-family.test.ts Assert the report template references gradeLevel and formatter still emits it.
sdks/typescript/src/evaluators/student-facing-text/ela-reading/vocabulary-complexity.ts Honor contract-declared grade conditions and FK preprocessing condition (skip FK for 5–12).
sdks/typescript/src/prompts/vocabulary-complexity/system.ts Route system prompt by contract condition rather than hardcoded grades.
sdks/typescript/src/prompts/vocabulary-complexity/user.ts Route user template + FK substitution by contract conditions and guard FK substitution.
sdks/typescript/tests/unit/prompts/vocabulary-complexity.test.ts Add contract-conformance tests for branch selection and FK binding behavior.
sdks/typescript/tests/unit/evaluators/vocabulary-complexity.test.ts Loosen mocked timing assertion to allow 0ms processing time.
sdks/typescript/src/evaluators/multi-step.ts Add requirePreprocessing helper for contract-derived preprocessing lookup.
sdks/typescript/src/features/readability.ts Update stale JSDoc about remaining PREPROCESSING_GAPS caller(s).

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread sdks/typescript/src/prompts/vocabulary-complexity/user.ts Outdated
Comment thread sdks/typescript/src/evaluators/multi-step.ts
@adnanrhussain
adnanrhussain merged commit ad8e537 into main Aug 30, 2026
16 checks passed
@adnanrhussain
adnanrhussain deleted the sdk-fix-four branch August 30, 2026 04:28
@czi-github-helper czi-github-helper Bot mentioned this pull request Aug 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants